Closed Bug 852044 Opened 7 years ago Closed 7 years ago

don't fire events from DocAccessible::ProcessLoad() sync

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(1 file)

this causes asserts because firing the event causes us to run script which tries to close a window which of course causes a reflow while NotificationController is processing updates.  This passes all tests, but I'm not sure hwo many asserts it fixes on !linux yet.
Attached patch patchSplinter Review
this is also good because it means these events participant in coalescence logic.
Attachment #726005 - Flags: review?(surkov.alexander)
(In reply to Trevor Saunders (:tbsaunde) from comment #1)

> this is also good because it means these events participant in coalescence
> logic.

only if busy state change event (if somebody plays with aria-busy on loading document)

[note to myself] so we just fire document load/state busy events after all events in the queue. Taking into account we could fire some events on the document content before this point then it shouldn't be bad.

Trev, do you think to do the same for generic notification processing?
Comment on attachment 726005 [details] [diff] [review]
patch

Review of attachment 726005 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Attachment #726005 - Flags: review?(surkov.alexander) → review+
> Trev, do you think to do the same for generic notification processing?

yes I think so
ewong's about to back this out for bustage, so I won't set target milestone, but I'll set assignee while I'm here. :D

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_scroll.xul | Test timed out

https://tbpl.mozilla.org/php/getParsedLog.php?id=20867947&tree=Mozilla-Inbound
Status: NEW → ASSIGNED
Jamie this will make it somewhat more likely that scrolling start will fire before document load (which could already happen in some cases)  Could that be a problem for you?
Flags: needinfo?(jamie)
It's not a problem if scrolling start fires before document load. However, it mustn't fire before initial focus.
Flags: needinfo?(jamie)
(In reply to James Teh [:Jamie] from comment #8)
> It's not a problem if scrolling start fires before document load. However,
> it mustn't fire before initial focus.

we preserve ordering of scroll start event wrt immediately preceeding focus event so this should be  ok.
then it makes change events order in the test
(In reply to alexander :surkov from comment #10)
> then it makes change events order in the test

wouldn't it be better  to make them asyncInvokerCheckers since they can technically race?
up to you
You need to log in before you can comment on or make changes to this bug.