The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla22

Status

()

Core
Disability Access APIs
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

unspecified
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 726005 [details] [diff] [review]
patch

this is also good because it means these events participant in coalescence logic.
Attachment #726005 - Flags: review?(surkov.alexander)

Comment 2

4 years ago
(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

4 years ago
Comment on attachment 726005 [details] [diff] [review]
patch

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

r=me
Attachment #726005 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 4

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

yes I think so
(Assignee)

Comment 5

4 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/221a7a558c63
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
(Assignee)

Comment 7

4 years ago
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)

Comment 8

4 years ago
It's not a problem if scrolling start fires before document load. However, it mustn't fire before initial focus.
Flags: needinfo?(jamie)
(Assignee)

Comment 9

4 years ago
(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

4 years ago
then it makes change events order in the test
(Assignee)

Comment 11

4 years ago
(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

4 years ago
up to you
(Assignee)

Comment 13

4 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2d1bfae2b4
(Assignee)

Comment 14

4 years ago
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
https://hg.mozilla.org/mozilla-central/rev/0a2d1bfae2b4
https://hg.mozilla.org/mozilla-central/rev/08c2a05be2e4
https://hg.mozilla.org/mozilla-central/rev/3825fdbcec62
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.