Closed Bug 297801 Opened 20 years ago Closed 19 years ago

[FIX]Replacing the document element via replaceChild() doesn't show new content

Categories

(Core :: DOM: Core & HTML, defect, P1)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9alpha1

People

(Reporter: martin.honnen, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: testcase, Whiteboard: fixes 1.8 branch regression bug 323745)

Attachments

(2 files)

Testcase
7.49 KB, text/html
Details
Proposed patch
4.70 KB, patch
peterv
: review+
jst
: superreview+
Details | Diff | Splinter Review 
The test case at
http://home.arcor.de/martin.honnen/mozillaBugs/domLevel2/replaceContentInFrameDoc.html
is a HTML frameset where the lower frame contains an XML document and the upper
frame a HTML document with a button which when clicked uses DOM scripting to
replace the document element of the XML document with new content.
This approach used to crash Mozilla 1.8 but since bug
https://bugzilla.mozilla.org/show_bug.cgi?id=297311 has been fixed there is
still the issue that the new content of the XML document is not displayed.
When running the test case in the DOM inspector the DOM inspector shows that the
nodes have been replaced in the DOM but the display of the XML document is not
updated properly.
Boris, when you are back and dealing with bug
https://bugzilla.mozilla.org/show_bug.cgi?id=297644 maybe you can also look into
why the new content of the XML document is not displayed.
It's probably not displayed because of bug 297644 -- we assert and bail out
instead of creating the new layout objects...
Depends on: 297644
So this is a regression from bug 167355.  When we remove the root node we set
mDidInitialReflow to false in the presshell.  Then when the new root is inserted
we think we haven't done initial reflow yet, so we bail out silently....

I suppose we could put random InitialReflow() calls in doInsertChildAt and
doReplaceChild, but is there a better solution here?
Flags: blocking1.9a1?
OS: Windows XP → All
Hardware: PC → All
Better testcase (from bug 322930) coming up.
Summary: when replacing the document element of an XML document rendered in a frame the new content is not displayed → Replacing the document element via replaceChild() doesn't show new content
*** Bug 322930 has been marked as a duplicate of this bug. ***
Attached file Testcase — 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed patch
        — 
          — Splinter Review
      

    


  
This removes the code added in bug 167355 and instead aims to make <html> less special by sending ContentInserted notifications for it.  This fixes this bug (since the presshell is at all times live), and doesn't regress bug 167355 (since frames get constructed for the <html> when it's parsed).

Normal pageload is completely unaffected, since the notification happens before InitialReflow() and hence is ignored.
Assignee: general → bzbarsky
Status: NEW → ASSIGNED

        Attachment #208100 -
        Flags: superreview?(jst)

        Attachment #208100 -
        Flags: review?(peterv)

    
  
Priority: -- → P1
Summary: Replacing the document element via replaceChild() doesn't show new content → [FIX]Replacing the document element via replaceChild() doesn't show new content
Target Milestone: --- → mozilla1.9alpha

    
    

    
  
Comment on attachment 208100 [details] [diff] [review]
Proposed patch

+          mNotifiedRootInsertion = PR_TRUE;
+          PRInt32 index = mDocument->IndexOf(mRoot);
+          NS_ASSERTION(index != -1, "mRoot not child of document?");
+          NotifyInsert(nsnull, mRoot, index);

Is there a reason mNotifedRootInsertion is set to true before the notification has actually been fired? Seems backwards, but I'm fine with it if there's a reason for it.

sr=jst

        Attachment #208100 -
        Flags: superreview?(jst) → superreview+

    
    

    
  
I set it up front so that we can't possibly reenter this code from the notification.  I don't think we can anyway, but just playing it safe.  I can document that if you'd like.

        Attachment #208100 -
        Flags: review?(peterv) → review+

    
    

    
  
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED

    
    

    
  
I am testing with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060113 Firefox/1.6a1 and with both test cases the new contents is being rendered.
However with <https://bugzilla.mozilla.org/attachment.cgi?id=208091> there is a lot of new contents displayed, more than fits into the current window, but the scrollbar is greyed out. Scrolling is only possible with the mouse wheel.
Should I file a new bug on the scroll (bar) issue or reopen this bug?
Other pages have scroll bars working just fine in that build so I think the scroll (bar) issue is related to the replaceChild on the documentElement.
Priority: P1 → --
Target Milestone: mozilla1.9alpha → ---

    
  
Priority: -- → P1
Target Milestone: --- → mozilla1.9alpha

    
    

    
  
Martin, that's bug 78070.

    
    

    
  
*** Bug 323745 has been marked as a duplicate of this bug. ***

    
  
Flags: blocking1.9a1?

    
    

    
  
Since a 1.8 regression (bug 323745) was duped to this one, should this fix land on the 1.8 branch as well?
Flags: blocking1.8.1?
Flags: blocking1.8.0.6?
Whiteboard: fixes 1.8 branch regression bug 323745

    
    

    
  
let's see if we can get this into 1.8.1beta2
Flags: blocking1.8.1? → blocking1.8.1+
Target Milestone: mozilla1.9alpha → mozilla1.8.1beta2

    
    

    
  
I'm not willing to land this patch on the branch without a large amount of branch testing and code analysis first.  I've made a number of changes to notifications for the root and its kids on trunk, and I'm really not sure which of the numerous changes are needed to make sure that this change works right.  This code is very fragile, and it's been broken for a very long time -- see bug 323745 comment 6 for the exact relationship between these two regressions (this one dating from 2002 or so, and quite present in the 1.8 branch).

If we feel that this is branch-critical, I can try to make the time to do the work above, I guess...  Or we can try to find someone else with a week or two to spare for it...

    
    

    
  
Minusing per comment 16 and because bug 323745 is a 1.8-branch regression *before* 1.8/Firefox 1.5 shipped.
Flags: blocking1.8.1+ → blocking1.8.1-

    
    

    
  
Following the 1.8.1 team's lead -- not critical for 1.8.0.x
Flags: blocking1.8.0.7? → blocking1.8.0.7-

    
  
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha

    
  
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: