Don't start firing visibility change events until we've updated state for the entire docshell tree involved

RESOLVED INVALID

Status

()

RESOLVED INVALID
6 years ago
6 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla20
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [leave open][spec test suite bogus])

Attachments

(1 attachment)

This preserves the fiction that the state is that of the root document, not of the individual documents, though that's obviously false.

Patch coming up that does the state updates sync on docshell active state change but keeps firing the events async.  This does mean there's a time when the state has changed but the event has not fired yet, but I don't think I really care...
(Assignee)

Comment 1

6 years ago
Created attachment 688609 [details] [diff] [review]
Don't fire visibility change events until we're done updating vsibility states in the entire docshell tree.
Attachment #688609 - Flags: review?(bugs)
(Assignee)

Updated

6 years ago
Whiteboard: [need review]
Comment on attachment 688609 [details] [diff] [review]
Don't fire visibility change events until we're done updating vsibility states in the entire docshell tree.

So the patch about updating the visibility state from docshell, not
so much about changing the firing of the events.

This needs some test.
Attachment #688609 - Flags: review?(bugs) → review+
(Assignee)

Comment 3

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/380e2b71cbcf with a test.
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla20
(Assignee)

Comment 4

6 years ago
That said, per the current processing model in the spec (but not the test suite!) our behavior was correct before this change.  Working on figuring out whether the bug is in the spec or in the tests.
(In reply to Boris Zbarsky (:bz) from comment #4)
> That said, per the current processing model in the spec (but not the test
> suite!) our behavior was correct before this change
Fun

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/380e2b71cbcf
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

6 years ago
I went ahead and backed this out, since our old behavior matches the spec and I don't want to ship a gratuitous change.  If the spec ever changes, I'll look at this again.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f93a72920e0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [leave open][spec test suite bogus]

Comment 8

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #7)
> I went ahead and backed this out, since our old behavior matches the spec
> and I don't want to ship a gratuitous change.  If the spec ever changes,
> I'll look at this again.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3f93a72920e0

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/3f93a72920e0
(Assignee)

Comment 9

6 years ago
Per mail in mailing list, this was just a test bug.  The spec, and our code, is correct.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.