Closed Bug 272646 Opened 15 years ago Closed 11 years ago

tabs insertBefore lastChild random insert

Categories

(Core :: XUL, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bugzillamozillaorg, Assigned: bzbarsky)

References

Details

(Keywords: testcase)

Attachments

(5 files)

689 bytes, application/vnd.mozilla.xul+xml
Details
761 bytes, application/vnd.mozilla.xul+xml
Details
808 bytes, application/vnd.mozilla.xul+xml
Details
5.59 KB, patch
bzbarsky
: review-
Details | Diff | Splinter Review
1.59 KB, application/vnd.mozilla.xul+xml
Details
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20041008
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20041008

The attached snipplet (follows) should append a <tab> to a tabbox, using
insertBefore(newnode,lastChild).

Problem: The new <tab> is inserted in a strange way. (First before first, then
before last, ..)

Checked in Firefox 1.0, Mozilla 1.8 + 1.7

Reproducible: Always
Steps to Reproduce:
1. open attached testcase
2. click 'add tab' a few times
3. profit ;-)

Actual Results:  
Tabs get inserted but not at expected position.

Expected Results:  
Insert the new <tab> element right before the last node in <tabs>.

Hope this is a coding error on my side ;-)

I put it on major - just because it's an issue with my xul app. If there's a
workaround, feel free to downgrade.
Attached file Testcase
foo = replaceChild(new,lastviaid); appendChild(foo);
doesn't work neither
old = removeChild(foo); appendChild(new); appendChild(old);
Works... seems like a workaround ;-)
Downgrading severity. Workaround found.
Severity: major → normal
I see the bug with the first testcase using linux trunk 2004120106, but
testcase2 worksforme.
Assignee: general → nobody
Status: UNCONFIRMED → NEW
Component: General → XP Toolkit/Widgets: XUL
Ever confirmed: true
Keywords: testcase
Product: Mozilla Application Suite → Core
QA Contact: general
Version: unspecified → Trunk
I have the same problem on a Windows platform, with Firefox 1.0, using 
insertBefore (to implement drag and drop).
With 3 tab elements A, B, C, removing C and then insertBefore B, C ends up 
before A. But the DOM explorator is OKAY !!! The DOM says C is between A and B.

In the code below, replace tab elements with textbox elements and all works 
fine when you click the button:

<?xml version="1.0"?>
<window id="reivax" title="reivax" windowtype="reivax:main"
        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <tabbox>
      <tabs id="t">
         <tab id="A" label="A"/>
         <tab id="B" label="B"/>
         <tab id="C" label="C"/>
      </tabs>
   </tabbox>
   <button onclick="go()" label="go"/>
   <script type="application/x-javascript">
      <![CDATA[
      function go() {
         var C = document.getElementById('C');
         var B = document.getElementById('B');
         var t = document.getElementById('t');
         
         var newTab = t.removeChild(C);
         newTab = t.insertBefore(newTab, B);
      }
      ]]>
   </script>
</window>
Attached patch Possible patchSplinter Review
This issue appears when you have:

<xul:box>
  <sometag/>
  <children/>
</xul:box>

The frame insertion code isn't accounting for <sometag> when inserting at the
insertion point.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #168228 - Flags: review?(bzbarsky)
Neil, it'll take me a few days to get to this... does this happen to fix bug
261826, too?
Heh. I thought I'd seen a similar bug before.

Yes, that bug is fixed by this patch too.
So... if you add the assert patch in that bug to your tree, does the assert fire
when opening menus from the menubar?

Note that it looks to me like either this patch or the documentation about what
this index is in nsIBindingManager is wrong...
Comment on attachment 168228 [details] [diff] [review]
Possible patch

Marking r- to get answer to question in comment 10.
Attachment #168228 - Flags: review?(bzbarsky) → review-
Blocks: 261826
Blocks: 286513
Depends on: 307394
Blocks: 309914
Blocks: 299586
Duplicate of this bug: 319808
It would be nice to fix this, but not sure if it's worth fixing this now, or waiting for xbl2. There are two issues, first is that the index passed to FindPrevious/NextSibling isn't accurate. That's bug 307394. The other is that the code in nsBindingManager::ContentAppended/ContentInserted doesn't create child lists properly when there are multiple insertion points. Not sure if there's a bug on that, but there are comments in the code to that effect.
Blocks: 374719
Something similar happends to tab elements that gets hidden and shown again. When a hidden tab gets shown again, it will be shown before it's previous sibling when beeing the last tab in the tabbox.
Current workaround: use "collapsed" instead of "hidden"
Hiding the last tab in the testcase and showing it again, moves the tab before it's original previous sibling.
For the "best experience", hide and show the "extension" tab first than hide/show the "overview" tab.
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
Blocks: 472020
Fixed by checkin for bug 307394.  Checked in test.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee: enndeakin → bzbarsky
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/39400e032624
Backed out changeset fc813bf68348 for failing reftest layout/reftests/bugs/272646-1.xul on OS X. r=backout
You need to log in before you can comment on or make changes to this bug.