Closed Bug 943804 Opened 6 years ago Closed 6 years ago

Some binding changes not properly reflected in event handling

Categories

(Core :: XBL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox27 --- fixed
firefox28 --- fixed

People

(Reporter: neil, Assigned: neil)

References

Details

(Keywords: regression, Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

When you change the binding on an element from one with anonymous content to one without anonymous content then the child nodes aren't told that they no longer have an XBL anonymous parent. This means that events dispatched to them end up in a disconnected node.
Attached patch Proposed patchSplinter Review
Always clear insertion parents of cleared inserted children, as they may not necessarily get reinserted under another insertion parent.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8339166 - Flags: review?(mrbkap)
Comment on attachment 8339166 [details] [diff] [review]
Proposed patch

Thanks for finding this.
Attachment #8339166 - Flags: review?(mrbkap) → review+
Blocks: 892638
https://hg.mozilla.org/mozilla-central/rev/a98e26325e28
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment on attachment 8339166 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 653881
User impact if declined: Bug 938447 is one known side-effect of this bug
Testing completed (on m-c, etc.): Fixes known test failures (bug 892638)
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #8339166 - Flags: approval-mozilla-beta?
Attachment #8339166 - Flags: approval-mozilla-aurora?
Comment on attachment 8339166 [details] [diff] [review]
Proposed patch

We're about to cut Beta over to Release and can't take this for a final beta (not sec-critical, not tracked, and already shipped without major user feedback) so approving for Aurora only.
Attachment #8339166 - Flags: approval-mozilla-beta?
Attachment #8339166 - Flags: approval-mozilla-beta+
Attachment #8339166 - Flags: approval-mozilla-aurora?
Attachment #8339166 - Flags: approval-mozilla-aurora+
Attachment #8339166 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Whiteboard: [qa-]
I reproduced the crash and determined that there is an additional problem. We are displaying the same image on the page twice, once as a WebGL texture with non-premultiplied alpha and once as a content image with premultiplied alpha. Because we only store one decoded version of the image at a time, whenever we try to draw the content version of the image it kicks out the WebGL version, and vice versa. This causes us to want to ForceDiscard() twice in a short span of time, but CanForciblyDiscard() doesn't make sure the image is fully decoded, so when we do so the second time we hit an assert in debug builds (because the decoder is still open) or a crash in release builds.

This patch just makes CanForciblyDiscard() return false if the image is not fully decoded.
Attachment #8350416 - Flags: review?(tnikkel)
Assignee: neil → seth
Comment on attachment 8350416 [details] [diff] [review]
Don't force discard images when they have a decoder open.

Argh. Wrong bug. Sorry.
Attachment #8350416 - Attachment is obsolete: true
Attachment #8350416 - Flags: review?(tnikkel)
Assignee: seth → neil
You need to log in before you can comment on or make changes to this bug.