Closed
Bug 626667
Opened 12 years ago
Closed 12 years ago
deal with cached accessible tree only
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla2.0b11
People
(Reporter: surkov, Assigned: surkov)
References
Details
(Keywords: access)
Attachments
(3 files)
4.70 KB,
patch
|
davidb
:
review+
davidb
:
approval2.0+
|
Details | Diff | Splinter Review |
2.39 KB,
patch
|
davidb
:
review+
davidb
:
approval2.0+
|
Details | Diff | Splinter Review |
3.00 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
the work has been started in bug 606924, however accessible tree traversal methods may trigger accessible tree update (like GetParent()) at unsafe time (for example when AT handles hide event for display: none element). We should deal with cached accessible tree only to accessible tree creation at wrong time.
Comment 1•12 years ago
|
||
This is a good idea but are you thinking this needs to make it into FF4?
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to comment #1) > This is a good idea but are you thinking this needs to make it into FF4? Yes, we need to get rid all cases when we may create accessible at wrong time.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #507113 -
Flags: review?(bolterbugz)
Attachment #507113 -
Flags: approval2.0?
Comment 4•12 years ago
|
||
Comment on attachment 507113 [details] [diff] [review] part1v1: getParent shouldn't repair the tree [landed 2011-01-29] This sort of blows my mind.
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #507132 -
Flags: review?(bolterbugz)
Attachment #507132 -
Flags: approval2.0?
Comment 6•12 years ago
|
||
We'll need QA of course.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to comment #3) > Created attachment 507113 [details] [diff] [review] > part1v1: getParent shouldn't repair the tree try server build http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/surkov.alexander@gmail.com-0a433462a83f
Comment 8•12 years ago
|
||
Comment on attachment 507113 [details] [diff] [review] part1v1: getParent shouldn't repair the tree [landed 2011-01-29] OK r=me pending QA. >- virtual nsAccessible* GetParent(); >+ nsAccessible* GetParent() const { return mParent; }
Attachment #507113 -
Flags: review?(bolterbugz) → review+
Comment 9•12 years ago
|
||
Comment on attachment 507132 [details] [diff] [review] part2v1: get rid GetCachedParent [landed 2011-01-29] r=me pending QA
Attachment #507132 -
Flags: review?(bolterbugz) → review+
Comment 10•12 years ago
|
||
Running with the try-server build for this patch, I got a non-reproducible crash: http://crash-stats.mozilla.com/report/index/bp-e32411a3-d44c-4a6a-be6f-6135d2110126 Performing the same steps didn't bring the crash a second time.
Updated•12 years ago
|
Attachment #507113 -
Flags: approval2.0? → approval2.0+
Updated•12 years ago
|
Attachment #507132 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to comment #10) > Running with the try-server build for this patch, I got a non-reproducible > crash: > http://crash-stats.mozilla.com/report/index/bp-e32411a3-d44c-4a6a-be6f-6135d2110126 no, stack trace unfortunately
Comment 12•12 years ago
|
||
Haven't seen another crash yet, and David and I agreed that it would be good to land this so if there is a crasher that's being introduced, we have a good chance of catching it in a regular nightly, with a valid stack trace.
Assignee | ||
Comment 13•12 years ago
|
||
ok, it's queued for landing
Assignee | ||
Comment 14•12 years ago
|
||
part1: getParent shouldn't repair the tree http://hg.mozilla.org/mozilla-central/rev/4b8113a528f0 part2: get rid GetCachedParent http://hg.mozilla.org/mozilla-central/rev/e006628cc089 landed on 2.0 beta 11
Assignee | ||
Updated•12 years ago
|
Attachment #507113 -
Attachment description: part1v1: getParent shouldn't repair the tree → part1v1: getParent shouldn't repair the tree [landed 2011-01-29]
Assignee | ||
Updated•12 years ago
|
Attachment #507132 -
Attachment description: part2v1: get rid GetCachedParent → part2v1: get rid GetCachedParent [landed 2011-01-29]
Comment 15•12 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessible.cpp#2894 2894 PRInt32 2895 nsAccessible::GetIndexInParent() 2896 { 2897 // XXX: call GetParent() to repair the tree if it's broken. 2898 GetParent(); 2899 return mIndexInParent; 2900 } Looks like this needs an update.
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to comment #15) > 2897 // XXX: call GetParent() to repair the tree if it's broken. > 2898 GetParent(); > 2899 return mIndexInParent; > 2900 } > > Looks like this needs an update. right, thank you!
Assignee | ||
Comment 17•12 years ago
|
||
Attachment #508229 -
Flags: review?(bolterbugz)
Comment 18•12 years ago
|
||
Comment on attachment 508229 [details] [diff] [review] patch3v1: rudiment of GetIndexInParent fix r=me. >-nsAccessible::GetIndexInParent() >+nsAccessible::GetIndexInParent() const > { >- // XXX: call GetParent() to repair the tree if it's broken. >- GetParent(); > return mIndexInParent; Great.
Attachment #508229 -
Flags: review?(bolterbugz) → review+
Assignee | ||
Comment 19•12 years ago
|
||
part3 is landed on 2.0 beta 11 - http://hg.mozilla.org/mozilla-central/rev/70b1c8dd220e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
You need to log in
before you can comment on or make changes to this bug.
Description
•