Bug 121602

Summary: AX: Accessibility methods using textUnderElement should be declared as non-const
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, eric.carlson, glenn, jdiggs, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 121558    
Attachments:
Description Flags
Patch proposal none

Mario Sanchez Prada
Reported 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
Attachments
Patch proposal (79.62 KB, patch)
2013-09-19 07:21 PDT, Mario Sanchez Prada 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-09-19 04:12:01 PDT
<rdar://problem/15029309>
Mario Sanchez Prada
Comment 2 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.
Mario Sanchez Prada
Comment 3 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
Mario Sanchez Prada
Comment 4 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.
Note You need to log in before you can comment on or make changes to this bug.