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

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

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.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: patch

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

Comment 2

[alexander :surkov (:asurkov)]

(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 3

[alexander :surkov (:asurkov)]

Comment on attachment 726005 [details] [diff] [review]
patch

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

r=me

Comment 4

[Trevor Saunders (:tbsaunde)]

> Trev, do you think to do the same for generic notification processing?

yes I think so

Comment 5

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/221a7a558c63

Comment 6

[Richard Newman [:rnewman]]

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

Comment 7

[Trevor Saunders (:tbsaunde)]

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?

Comment 8

[James Teh [:Jamie]]

It's not a problem if scrolling start fires before document load. However, it mustn't fire before initial focus.

Comment 9

[Trevor Saunders (:tbsaunde)]

(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.

Comment 10

[alexander :surkov (:asurkov)]

then it makes change events order in the test

Comment 11

[Trevor Saunders (:tbsaunde)]

(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?

Comment 12

[alexander :surkov (:asurkov)]

up to you

Comment 13

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2d1bfae2b4

Comment 14

[Trevor Saunders (:tbsaunde)]

kill more expected assertion stuff
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/08c2a05be2e4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3825fdbcec62

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0a2d1bfae2b4
https://hg.mozilla.org/mozilla-central/rev/08c2a05be2e4
https://hg.mozilla.org/mozilla-central/rev/3825fdbcec62