The default bug view has changed. See this FAQ.

de-ns-ify nsHTMLTextAccessible file classes

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: capella)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
how about to rename the files to HTMLElementAccessibles.h/cpp ('s' on the end)? As follow up we can do that for base/nsBaseWidgetAccessible and xul/nsXULTextAccessible?
(Assignee)

Updated

5 years ago
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
(Reporter)

Comment 1

5 years ago
Trevor, ping?
(Assignee)

Comment 2

5 years ago
Will we rename nsHTMLTextAccessible ...

contains two classes derived from nsLeafAccessible: 
class nsHTMLHRAccessible
class nsHTMLBRAccessible

and two classes derived from nsLeafAccessible: 
nsHyperTextAccessibleWrap
class nsHTMLLabelAccessible
class nsHTMLOutputAccessible

? Just remove the "ns" portion of the class names? Something further?
(In reply to alexander :surkov from comment #1)
> Trevor, ping?

This doesn't seem like a high priority, comment 0 seems reasonable.  I'm not terribly a fan of html/HTMLBlah.{h,cpp} but the way do includes I don't have a better idea.
(Reporter)

Comment 4

5 years ago
(In reply to Mark Capella [:capella] from comment #2)
> Will we rename nsHTMLTextAccessible ...

yes (per comment #3)

> contains two classes derived from nsLeafAccessible: 
> class nsHTMLHRAccessible
> class nsHTMLBRAccessible
> 
> and two classes derived from nsLeafAccessible: 
> nsHyperTextAccessibleWrap
> class nsHTMLLabelAccessible
> class nsHTMLOutputAccessible
> 
> ? Just remove the "ns" portion of the class names? Something further?

yes/no
(Assignee)

Comment 5

5 years ago
thank you.
(Assignee)

Comment 6

5 years ago
Created attachment 629110 [details] [diff] [review]
Patch (v1)

Initial portion of the patch for review ...

For the followup:

  base/nsBaseWidgetAccessible will become base/BaseElementAccessibles?
    (dropping the "ns" from class nsLeafAccessible, class nsLinkableAccessible, and class nsEnumRoleAccessible)

  xul/nsXULTextAccessible will become xul/XULElementAccessibles?
     (dropping the "ns" from class nsXULTextAccessible, class nsXULTooltipAccessible, and class nsXULLinkAccessible)

And might you to go ahead and incorporate those changes here?
Attachment #629110 - Flags: review?(surkov.alexander)
(Reporter)

Comment 7

5 years ago
Mark, please put them into mozilla::a11y namespace, it should be easy.
(Assignee)

Comment 8

5 years ago
Created attachment 629126 [details] [diff] [review]
Patch (v2)

Sure was! The classes with casting methods such as AsRoot(), AsDoc(), etc. are the ones that seem to get complicated. (Though I've done a couple of those without problems).

I'm still working to understand that.
Attachment #629110 - Attachment is obsolete: true
Attachment #629110 - Flags: review?(surkov.alexander)
(Reporter)

Comment 9

5 years ago
btw, don't forget to remove namespace stuffs like mozilla::a11y::role
(Assignee)

Comment 10

5 years ago
I've tried to remove mozilla::a11y::role from virtual mozilla::a11y::role NativeRole(); before but had trouble there also,  ... 

mxr.mozilla.org/mozilla-central/source/accessible/src/base/Role.h#10

Shows the enum in the roles namespace, which is like "under" a11y, so maybe it's just a matter of adding
namespace roles {
...
}
for that ?

Let me try it ...
(Reporter)

Comment 11

5 years ago
use a11y::role if role is ambiguous.
(Assignee)

Comment 12

5 years ago
Yep, that's what it took ... rebuilding / testing ....
(Assignee)

Comment 13

5 years ago
Created attachment 629141 [details] [diff] [review]
Patch (v3)

Latest and greatest ...
Attachment #629126 - Attachment is obsolete: true
(Reporter)

Comment 14

5 years ago
Comment on attachment 629141 [details] [diff] [review]
Patch (v3)

Review of attachment 629141 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/html/nsHTMLTextAccessible.h
@@ +19,3 @@
>  {
>  public:
> +  HTMLHRAccessible(nsIContent* aContent, DocAccessible* aDoc);

if you don't mind then you can move implementation here hoping it's get inlined (same for other classes)
Attachment #629141 - Flags: review+
(Assignee)

Comment 15

5 years ago
Created attachment 629173 [details] [diff] [review]
Patch (final)

As pushed to try, headed to inbound eventually, see attached

Try :
https://tbpl.mozilla.org/?tree=Try&rev=da1838cb71aa
(Assignee)

Comment 16

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d2624fac9886
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/b3856cc3e431
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.