Last Comment Bug 762394 - replace do_QueryObject() with static cast in CAccessibleText
: replace do_QueryObject() with static cast in CAccessibleText
Status: RESOLVED FIXED
[good first bug][mentor=trev.saunders...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Andrew Quartey [:drexler]
:
Mentors:
Depends on:
Blocks: 762038
  Show dependency treegraph
 
Reported: 2012-06-07 00:24 PDT by Trevor Saunders (:tbsaunde)
Modified: 2012-06-16 06:50 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (16.30 KB, patch)
2012-06-14 11:22 PDT, Andrew Quartey [:drexler]
no flags Details | Diff | Splinter Review
patch (21.12 KB, patch)
2012-06-14 18:24 PDT, Andrew Quartey [:drexler]
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review
updated_patch (22.49 KB, patch)
2012-06-15 09:19 PDT, Andrew Quartey [:drexler]
no flags Details | Diff | Splinter Review

Description Trevor Saunders (:tbsaunde) 2012-06-07 00:24:40 PDT
nsRefPtr<HypertextAccessible> foo = do_QueryObject(this); should become HypertextAccessible* foo = static_cast<HypertextAccessibleWrap*>(this);
also rename it to ia2Text
Comment 1 Trevor Saunders (:tbsaunde) 2012-06-07 01:35:02 PDT
> also rename it to ia2Text

actually, ia2AccessibleText
Comment 2 Andrew Quartey [:drexler] 2012-06-14 11:22:22 PDT
Created attachment 633202 [details] [diff] [review]
patch

passed tests locally.
Comment 3 Trevor Saunders (:tbsaunde) 2012-06-14 15:00:54 PDT
Comment on attachment 633202 [details] [diff] [review]
patch

> #include "HyperTextAccessible.h"
>+#include "HyperTextAccessibleWrap.h"

HypertextAccessibleWrap.h is enough

> {
> __try {
>-  nsRefPtr<HyperTextAccessible> textAcc(do_QueryObject(this));
>-  if (textAcc->IsDefunct())
>+  HyperTextAccessible* ia2AccessibleText = static_cast<HyperTextAccessibleWrap*>(this);

same as bug 762396 rename the class not variable names :)  Sorry I wasn't clearer.

otherwise this one seems fine too
Comment 4 Andrew Quartey [:drexler] 2012-06-14 18:24:00 PDT
Created attachment 633349 [details] [diff] [review]
patch

updated patch addressing comments.
Comment 5 Trevor Saunders (:tbsaunde) 2012-06-15 04:42:30 PDT
Comment on attachment 633349 [details] [diff] [review]
patch

># HG changeset patch
># Parent e116a5da6cedfe7f980979005c5f4eaa4e0bfa4c
># User Andrew Quartey <andrew.quartey@gmail.com>
>Bug 762394 - Replace do_QueryObject() with static_cast in CAccessibleText
>
>diff --git a/accessible/src/msaa/HyperTextAccessibleWrap.h b/accessible/src/msaa/HyperTextAccessibleWrap.h
>--- a/accessible/src/msaa/HyperTextAccessibleWrap.h
>+++ b/accessible/src/msaa/HyperTextAccessibleWrap.h
>@@ -4,17 +4,17 @@
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #ifndef mozilla_a11y_HyperTextAccessibleWrap_h__
> #define mozilla_a11y_HyperTextAccessibleWrap_h__
> 
> #include "HyperTextAccessible.h"
>-#include "CAccessibleText.h"
>+#include "ia2AccessibleText.h"
> #include "CAccessibleEditableText.h"
> #include "ia2AccessibleHyperText.h"

you should keep them in order, but actually I don't think you need to include it here at all.

> STDMETHODIMP
>-CAccessibleText::get_attributes(long aOffset, long *aStartOffset,
>+ia2AccessibleText::get_attributes(long aOffset, long *aStartOffset,
>                                 long *aEndOffset, BSTR *aTextAttributes)

nit, you break alignment here, and other methods with args on a second line.

> {
Comment 6 Andrew Quartey [:drexler] 2012-06-15 09:19:27 PDT
Created attachment 633552 [details] [diff] [review]
updated_patch

Updated patch addressing nits.
Comment 7 Geoff Lankow (:darktrojan) 2012-06-15 22:54:58 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/400bc23212f7
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:50:12 PDT
https://hg.mozilla.org/mozilla-central/rev/400bc23212f7

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