Closed Bug 1026008 Opened 6 years ago Closed 6 years ago
XBL child nodes not using document order on Win64 self-build
[I dogfood with a Win64 self-build. If I use a Win32 self-build on the same profile, I don't observe the issue. I don't know why there would be a difference. I haven't tried a Linux64 build yet.] SeaMonkey has a custom Preferences window which is loosely based around the Toolkit preferences code but dynamically creates its <prefpane> elements from its preferences tree. Lightning includes an overlay for the window which statically adds its <prefpane> elements. These therefore precede the dynamically created <prefpane> elements. The <prefpane> elements are then inserted under a <xul:deck> element in SeaMonkey's prefwindow binding using <children includes="prefpane">. On a 32-bit build this works as expected: the Lightning <prefpane> elements appear first, then the dynamically created <prefpane> elements. This means that you can simply set the <deck>'s selected index to the <prefpane>'s index in the DOM. However on a 64-bit build Lightning's <prefpane> elements appear after the dynamically created <prefpane> elements! Hilarity ensues. Note that you can use DOM Inspector to "fix" the problem by deleting Lightning's <prefpane> elements from the DOM and then undoing the deletion. This then causes the elements to be re-inserted in the correct order under the <deck>.
Aha! This is a regression from bug 1004098. It changed the type of a variable from uint32_t to size_t, although it's only compared to other uint32_t variables.
CC'ing dmajor just in case he's staring at weird Win64 test failures dealing with XBL...
Neil do you know where the bug is or does this still need investigation?
Yes, I've already written a quick hack but I haven't had time to finish polishing it yet.
So, what's my try chooser syntax to test my patch to see whether it fixes bug 1024269?
You could test locally, just set the pref dom.allow_XUL_XBL_for_file to true and load layout/reftests/dom/multipleinsertionpoints-appendsingle-2.xhtml trychooser lets you choose win64 http://trychooser.pub.build.mozilla.org/ but I've never tried it.
This works but I don't really like it.
I tried making everything consistently size_t but there was too much unrelated 32 bit action going on for me to be sure that I was getting it right. While I was there I replaced all uses of mInsertedChildren with the relevant public accessors which meant I could make the member private and remove the friends. It also improves the readability of the code.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8441957 - Flags: review?(mrbkap)
Comment on attachment 8441957 [details] [diff] [review] Proposed patch Review of attachment 8441957 [details] [diff] [review]: ----------------------------------------------------------------- I guess you could have also used a static class member to get the explicit type right? Those stink, though, so r=me.
Attachment #8441957 - Flags: review?(mrbkap) → review+
Backed out this and bug 1025264 in https://hg.mozilla.org/integration/mozilla-inbound/rev/b85caf0e485a for debug crashtest leaks: https://tbpl.mozilla.org/php/getParsedLog.php?id=42093698&tree=Mozilla-Inbound
(In reply to Wes Kocher from comment #13) > Backed out this and bug 1025264 Actually it was 1026254... so it needed to be backed out for wrong bug# anyway.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8441957 [details] [diff] [review] Proposed patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 1004098 User impact if declined: None, we don't release win64 builds Testing completed (on m-c, etc.): Fixes tests Risk to taking this patch (and alternatives if risky): String or IDL/UUID changes made by this patch: None
Attachment #8441957 - Flags: approval-mozilla-aurora?
Comment on attachment 8441957 [details] [diff] [review] Proposed patch Aurora- as I don't there is a need to take this change on Aurora. (As stated, we don't ship Win64 builds.)
Attachment #8441957 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.