759310 - de-ns-ify nsHTMLTextAccessible file classes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

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?

Comment 1

[alexander :surkov (:asurkov)]

Trevor, ping?

Comment 2

[Mark Capella [:capella]]

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?

Comment 3

[Trevor Saunders (:tbsaunde)]

(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.

Comment 4

[alexander :surkov (:asurkov)]

(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

Comment 5

[Mark Capella [:capella]]

thank you.

Comment 6

[Mark Capella [:capella]]

Attachment: 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?

Comment 7

[alexander :surkov (:asurkov)]

Mark, please put them into mozilla::a11y namespace, it should be easy.

Comment 8

[Mark Capella [:capella]]

Attachment: 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.

Comment 9

[alexander :surkov (:asurkov)]

btw, don't forget to remove namespace stuffs like mozilla::a11y::role

Comment 10

[Mark Capella [:capella]]

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 ...

Comment 11

[alexander :surkov (:asurkov)]

use a11y::role if role is ambiguous.

Comment 12

[Mark Capella [:capella]]

Yep, that's what it took ... rebuilding / testing ....

Comment 13

[Mark Capella [:capella]]

Attachment: Patch (v3)

Latest and greatest ...

Comment 14

[alexander :surkov (:asurkov)]

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)

Comment 15

[Mark Capella [:capella]]

Attachment: Patch (final)

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

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

Comment 16

[Mark Capella [:capella]]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d2624fac9886

Comment 17

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/b3856cc3e431