Closed Bug 561734 Opened 10 years ago Closed 10 years ago

Use do_QueryObject

Categories

(Core :: Disability Access APIs, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

Details

Attachments

(2 files)

do_QueryObject replaces:
nsAccUtils::QueryAccessible (2x)
nsAccUtils::QueryAccessibleTable
nsAccUtils::QueryAccessibleDocument (2x)
nsAccUtils::QueryAccessibleTree
Attached patch Proposed patchSplinter Review
Trying multiple addresses in case of mismatches, please cancel incorrect ones.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #441473 - Flags: review?(surkov.alexander)
Attachment #441473 - Flags: review?(marco.zehe)
Attachment #441473 - Flags: review?(bolterbugz)
(In reply to comment #1)
> Created an attachment (id=441473) [details]
> Proposed patch
> 
> Trying multiple addresses in case of mismatches, please cancel incorrect ones.

Neil, could you describe this more please? I do not see anything wrong with the patch.
Comment on attachment 441473 [details] [diff] [review]
Proposed patch

Does this have any expected impact? In other words, should I test this, or is this just code cleanup/refactor?
(In reply to comment #3)
> (From update of attachment 441473 [details] [diff] [review])
> Does this have any expected impact? In other words, should I test this, or is
> this just code cleanup/refactor?

It's cleanup.
Comment on attachment 441473 [details] [diff] [review]
Proposed patch

Cancelling my review request.
Attachment #441473 - Flags: review?(marco.zehe)
(In reply to comment #2)
> (In reply to comment #1)
> > Trying multiple addresses in case of mismatches, please cancel incorrect ones.
> Neil, could you describe this more please? I do not see anything wrong with the
> patch.
I wasn't sure who to request or whether I had remembered the correct email addresses (Bugzilla doesn't give you a chance to correct them when you're creating an attachment) so I simply typed in what I could remember.
I see :)
Comment on attachment 441473 [details] [diff] [review]
Proposed patch

r=me, thank you for fixing this and thank you for do_QueryObject implementation!
Attachment #441473 - Flags: review?(surkov.alexander) → review+
Comment on attachment 441473 [details] [diff] [review]
Proposed patch

r=me, nice 1 thanks.
Attachment #441473 - Flags: review?(bolterbugz) → review+
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/2d1edfb3aaa3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attached patch Missed someSplinter Review
Sorry for overlooking these. I wonder whether there are any more?
Attachment #444252 - Flags: review?(surkov.alexander)
Attachment #444252 - Flags: review?(bolterbugz)
Comment on attachment 444252 [details] [diff] [review]
Missed some


> nsTextAccessible::AppendTextTo(nsAString& aText, PRUint32 aStartOffset, PRUint32 aLength)
> {
>   nsIFrame *frame = GetFrame();
>-  NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
>+  if (!frame) return NS_ERROR_FAILURE;//NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);

why is this change?

r=me, thank you for doing this

I think there are no other cases for do_QueryObject
Attachment #444252 - Flags: review?(surkov.alexander) → review+
(In reply to comment #12)
> >-  NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
> >+  if (!frame) return NS_ERROR_FAILURE;//NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
> why is this change?
I've got no idea why that change is there at all. I can only guess it must have been left over from a review of some unrelated patch. Sorry about that.
Comment on attachment 444252 [details] [diff] [review]
Missed some

r=me thanks!
Attachment #444252 - Flags: review?(bolterbugz) → review+
Pushed changeset ce9f07472020 to mozilla-central.
You need to log in before you can comment on or make changes to this bug.