Bug 121602 - AX: Accessibility methods using textUnderElement should be declared as non-const
Summary: AX: Accessibility methods using textUnderElement should be declared as non-const
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 121558
  Show dependency treegraph
 
Reported: 2013-09-19 04:11 PDT by Mario Sanchez Prada
Modified: 2013-09-23 08:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch proposal (79.62 KB, patch)
2013-09-19 07:21 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2013-09-19 04:11:27 PDT
Since bug 120891 got fixed, the layout will always be updated by just using a TextIterator (it happens in the constructor), which is used from AccessibilityRenderObject::textUnderElement() when dealing with text objects. This means that calls to that method can potentially alter the accessibility tree and thus other methods relying on this should be considered as non-const.

See a longer explanation in https://bugs.webkit.org/show_bug.cgi?id=121558#c5
Comment 1 Radar WebKit Bug Importer 2013-09-19 04:12:01 PDT
<rdar://problem/15029309>
Comment 2 Mario Sanchez Prada 2013-09-19 07:21:24 PDT
Created attachment 212066 [details]
Patch proposal

Now attaching a patch proposal for this. I'm not setting the r? flag yet because, even if I was very careful doing the changes, I'm not entirely sure I haven't missed something that might showup in builds for other platform.

So, let's wait for the EWS first.
Comment 3 Mario Sanchez Prada 2013-09-19 07:57:01 PDT
Comment on attachment 212066 [details]
Patch proposal

So it seems my replace work + post processing work with grep did work fine, since EWS seems to be happy :)

Asking for review now, then
Comment 4 Mario Sanchez Prada 2013-09-23 08:27:52 PDT
After to talking to Chris by email, it seems that a better way to proceed here is to make sure we call AccessibilityObject::updateBackingStore() from the wrapper layer, so we don't have any outdated layout once we call any of the functions that might end up calling textUnderElement, so it's safe and won't change the accessibility tree.

So, I'm resolving this bug as invalid, and concentrating myself in fixing bug 121558, by implementing Chris's suggestion in the ATK layer too.