Last Comment Bug 759307 - rename nsHTMLImageAccessible to ImageAccessible
: rename nsHTMLImageAccessible to ImageAccessible
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Mark Capella [:capella]
:
Mentors:
Depends on:
Blocks: densifya11y
  Show dependency treegraph
 
Reported: 2012-05-29 05:47 PDT by alexander :surkov
Modified: 2012-06-02 12:29 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (28.39 KB, patch)
2012-06-01 18:58 PDT, Mark Capella [:capella]
surkov.alexander: review+
Details | Diff | Splinter Review
Patch (v2) (28.74 KB, patch)
2012-06-01 20:57 PDT, Mark Capella [:capella]
no flags Details | Diff | Splinter Review

Description alexander :surkov 2012-05-29 05:47:46 PDT
1) rename nsHTMLImageAccessible to ImageAccessible
2) move it to mozilla::a11y namespace
3) move files to generic folder
Comment 1 Mark Capella [:capella] 2012-06-01 18:58:20 PDT
Created attachment 629426 [details] [diff] [review]
Patch (v1)

Renamed, moved to /generic, header guards updated, and moved to namespace.
Comment 2 alexander :surkov 2012-06-01 19:32:09 PDT
Comment on attachment 629426 [details] [diff] [review]
Patch (v1)

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

r=me, nice

::: accessible/src/html/nsHTMLImageAccessible.h
@@ +30,5 @@
>  
>    // nsIAccessible
>    NS_IMETHOD GetActionName(PRUint8 aIndex, nsAString& aName);
>    NS_IMETHOD DoAction(PRUint8 index);
>  

somewhere here you forgot about mozilla::a11y::role

::: accessible/src/msaa/ia2AccessibleImage.cpp
@@ +9,5 @@
>  
>  #include "AccessibleImage_i.c"
>  
> +#include "ImageAccessible.h"
> +#include "ImageAccessibleWrap.h"

it's enough include wrap file only

@@ +46,5 @@
>  __try {
>    *aDescription = NULL;
>  
> +  ImageAccessibleWrap* acc =
> +    static_cast<ImageAccessibleWrap*>(this);

keep on the same line

@@ +75,5 @@
>    *aX = 0;
>    *aY = 0;
>  
> +  ImageAccessibleWrap* imageAcc =
> +    static_cast<ImageAccessibleWrap*>(this);

same

@@ +105,5 @@
>    *aHeight = 0;
>    *aWidth = 0;
>  
> +  ImageAccessibleWrap* imageAcc =
> +    static_cast<ImageAccessibleWrap*>(this);

and here
Comment 3 Mark Capella [:capella] 2012-06-01 20:41:34 PDT
FYI, for accessible.h and nsHTMLImageAccessible.h, changing mozilla::a11y::role to role causes a type def not found, and using a11y::role complains a11y is not a namespace, so I had to leave it as-is.

I was able to change ImageAccessible.h though.
Comment 4 alexander :surkov 2012-06-01 20:46:13 PDT
(In reply to Mark Capella [:capella] from comment #3)
> FYI, for accessible.h and nsHTMLImageAccessible.h, changing
> mozilla::a11y::role to role causes a type def not found, and using
> a11y::role complains a11y is not a namespace, so I had to leave it as-is.

> I was able to change ImageAccessible.h though.

it's true for Accessible.h since it's not in a11y namespace.

I didn't get about nsHTMLImageAccessible.h and ImageAccessible.h since it's the same but if I read you right then in former case you didn't change it but in the second case you did that.
Comment 5 Mark Capella [:capella] 2012-06-01 20:48:42 PDT
Yes.
Comment 6 alexander :surkov 2012-06-01 20:52:16 PDT
(In reply to Mark Capella [:capella] from comment #5)
> Yes.

sorry, yes what?
Comment 7 Mark Capella [:capella] 2012-06-01 20:57:07 PDT
Created attachment 629434 [details] [diff] [review]
Patch (v2)

I wasn't able to change nsHTMLImageAccessible.h, but I able to for ImageAccessible.h.
Comment 8 Mark Capella [:capella] 2012-06-01 21:03:26 PDT
Yes sorry, I'm just confusing you here. Same one before/after, see the diff for clarity.
Comment 9 alexander :surkov 2012-06-01 21:10:35 PDT
(In reply to Mark Capella [:capella] from comment #7)
> Created attachment 629434 [details] [diff] [review]
> Patch (v2)
> 
> I wasn't able to change nsHTMLImageAccessible.h, but I able to for
> ImageAccessible.h.

ok, got it. you did a build in the middle of progress or try to change that previously.
Comment 10 Mark Capella [:capella] 2012-06-02 01:15:50 PDT
TRY Push
https://tbpl.mozilla.org/?tree=Try&rev=07d55b4ddc69
Comment 11 Mark Capella [:capella] 2012-06-02 05:14:33 PDT
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d2624fac9886

Note You need to log in before you can comment on or make changes to this bug.