bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th from 16:00 until 20:00 UTC.

[FIX]Saving/Publishing page results in a crash - Trunk [@ nsComboboxControlFrame::ReflowCombobox]

VERIFIED FIXED in mozilla1.4beta


15 years ago
13 years ago


(Reporter: Chris Petersen, Assigned: bz)


(4 keywords)

crash, regression, testcase, topcrash+
Dependency tree / graph
Bug Flags:
blocking1.4a -

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [adt1], crash signature)


(5 attachments, 2 obsolete attachments)



15 years ago
Build: 2003-03-13-03
Platform: All
Expected Results: Page should be published to server
What I got: Application crashs during the publishing process
Steps to reproduce:

1) Go to any bugzilla bug report like this one
2) Select Edit Page from File menu
3) Select Publish As
4) Specify a ftp address, site name, page title, and file name.
5) Publish document
6) Publishing to address dialog appears
7) Notice mozilla-banner.gif is transfered but crash occurs while transfer html
and css files.

Comment 1

15 years ago
Created attachment 117552 [details]
stack trace

Comment 2

15 years ago
This problem is also occuring on a win32 trunk build (2003-03-13-04) under
Windows XP.

Comment 3

15 years ago
Chris--What ftp server are you trying to publish to?  Is it possible to reduce
this to a more minimal testcase (can you remove all but a couple things and
still crash?)
Keywords: crash, nsbeta1
OS: MacOS X → All

Comment 4

15 years ago
Sure. I'm planning to create a reduced test case of this page.
OS: All → MacOS X

Comment 5

15 years ago
I'm guessing this is related to or a duplicate of the other ftp bug on Darin's

220 geckoqa Microsoft FTP Service (Version 5.0).
Remote system type is Windows_NT.
Summary: Publishing page results in a crash → ftp Publishing page results in a crash (MS ftp server)

Comment 6

15 years ago
Editor triage team: nsbeta1+/adt1
Assignee: composer → darin
Keywords: nsbeta1 → nsbeta1+, regression
Whiteboard: [adt1]

Comment 7

15 years ago
Ok, It took awhile to narrow it down but I finally have a reduced test case :).

Comment 8

15 years ago
Created attachment 117789 [details]
Reduced test case 

This test case uses a LINK and SELECT elements. The LINK element references the
original css with a absolute path. The SELECT element contains a single OPTION
element. If you open this test case in Composer and publish it, a crash should

Comment 9

15 years ago
You can reproduce the crash if both the test case and the css file are stored
locally. The CSS file can actually be a empty file but needs to be in a separate
folder (not in the same directory as the test case). 

Comment 10

15 years ago
thx for the reduced testcase.
Severity: normal → critical
Flags: blocking1.4a?
Priority: -- → P1
Target Milestone: --- → mozilla1.4alpha

Comment 11

15 years ago
the stack trace doesn't seem to have anything to do with networking.  cc'ing bz
to help triage.
Is there an FTP server I could use to repro this?

Comment 13

15 years ago
hm...I'm getting a slightly differenet stack in the debugger but the cause of my
crash is because nsRuleNode::mPresContext is a bogus pointer (0xddddd) possibly
because nsStyleContext::mRuleNode is bogus. cc:ing more layout folks
hmm.. style contexts hold strong refs to rulenodes....  Is someone
over-releasing a rulenode somewhere?
The nsStyleContext -> nsRuleNode reference is through a GC (see bug 117316), not
reference counting (which it hasn't been since nsIRuleNode was removed).

Comment 16

15 years ago
-> default owner
Assignee: darin → composer

Comment 17

15 years ago
Chris--can you confirm that the same crash results if you publish with http or
saveAs to a new location?

Comment 18

15 years ago
Yes, I get the same stack trace (as attached) when either saving the test case
(Save as)  or using http:// to publish it to the server. Tested with Mach-O
2003-03-20-03 trunk build.


15 years ago
Summary: ftp Publishing page results in a crash (MS ftp server) → Saving/Publishing page results in a crash


15 years ago
Flags: blocking1.4a? → blocking1.4a-

Comment 19

15 years ago
-->Adam Lock since this also affects saving

Chris--does saving in the browser also crash or only in composer?
Assignee: composer → adamlock
OS: MacOS X → All

Comment 20

15 years ago
Created attachment 119293 [details] [diff] [review]
stack trace on Windows XP

I was able to reproduce this problem on XP as well.

It is not obvious what the cause might be, since it isn't happening anywhere in
webbrowser persist, but in the layout code. Perhaps there is a refcount issue
somewhere, either in the layout, or the upload section of persist.

I will see if I can pin this down further.

Comment 21

15 years ago
After talking to Kathy, one avenue of investigation looks promising.

The document is published via the persist object and specifying the
PERSIST_FLAGS_FIXUP_ORIGINAL_DOM flag. This means that during upload the
editor's copy of the dom document is being fixed up and the LINK's href
attribute will be changed to point to the CSS which has just been or is in the
process of being uploaded.

The next time the page is repainted the crash occurs.

It's possible that changing the href on the link might be causing some edge case
issue where style data is not being correctly cleaned out and reloaded. I'll see
if I can confirm this idea.

Comment 22

15 years ago
This bug probably belongs to Boris.

Bug is definitely caused by fixing up the original DOM. If I blank out that flag
uploading happens properly.

In the reduced test case above, upload sets the link element's new href to
"edit_bug.css" which propogates through the DOM code resulting in
nsStyleLinkElement::UpdateStyleSheet being called. This method sees the relative
link and resolves it to "http://bugzilla.mozilla.org/edit_bug.css". At this
point CSSLoaderImpl::LoadStyleLink tries to load the url synchronously, gets an
http error and during or after this it appears to leave the style rules in an
inconsistent state. The next repaint crashes it.

I'm going to reassign to Boris for the time being. It pretty much looks like a
CSS thing at this stage.

Note that even if this bug is fixed, the page css may be broken after upload
since it's resolved incorrectly. Perhaps telling persist to fixup the original
dom is not appropriate when editing non-local files?
Assignee: adamlock → bzbarsky
> At this point CSSLoaderImpl::LoadStyleLink tries to load the url synchronously

Um?  LoadStyleLink never does synchronous loading...

So there are at least 3 separate bugs here:

1) The wrong url is used (Adam's turf ;) )
2) The sheet is loaded at all
3) We crash.

At a guess, it looks like #2 is caused by the way CloneNode is implemented on
Link elements.  I'd say we want to disable updates before copying over all the
attributes.  Peter?  What do you think?

#3 is most likely caused by bug 85631

In any case, how do I reproduce this?  I do not have access to any FTP servers
allowing upload that I'm aware of.
Depends on: 85631
Priority: P1 → P2
Target Milestone: mozilla1.4alpha → mozilla1.4beta

Comment 24

15 years ago
bz--you don't need to publish, saving locally also crashes (in Composer).

Comment 25

15 years ago
Kathy- The problem only happens when saving or publishing this test case with
Composer. This same test case is properly saved when using the browser. Tested
with the 2003-04-02-03 Mach0 trunk build.
Is CloneNode really starting the load? IIRC we don't try to load until putting
the node in the document.
Well, I'm confused.  Since PERSIST_FLAGS_FIXUP_ORIGINAL_DOM is set, we don't
clone the node -- we modify it in-place (and yes, CloneNode does not load the
sheet; sorry, Peter).

I have a fix for issue #2 (I _think_ we want to fix it... but do we?). 
Attaching that.  Still looking into issue #3.
Created attachment 119316 [details] [diff] [review]
don't load the new sheet....
Attachment #119316 - Flags: superreview+

Comment 29

15 years ago
Perhaps I was confused about the synchronous thing, but I was looking at the
#ifdef ALLOW_ASYNCH_STYLE_SHEETS at the top of nsStyleLinkElement.cpp. Since
that's not defined anywhere I thought the default blocking mode as synchronous.

As for the CloneNode issue, I don't think it matters for persistence since if
you're cloning nodes during persistence you won't be rendering them anyway.
Priority: P2 → P1
Target Milestone: mozilla1.4beta → mozilla1.4alpha

Comment 30

15 years ago
Restoring priority / target.
Priority: P1 → P2
Target Milestone: mozilla1.4alpha → mozilla1.4beta
So what I am seeing is a _single_ call into UpdateStyleSheet, followed by a
crash on the following reflow.  So bug 85631 is not likely to be a problem.
Created attachment 119346 [details]
Really minimal testcase

Seems to only be a problem with comboboxes.... in a debug build I get:

frame: Block(-1) (0x870eaf4) style: 0x870eb74
:-moz-display-comboboxcontrol-frame {}
Wrong parent style context:  style: 0x870e394 {}
should be using:  style: 0x872bf58 {}
Style contexts are not in the same style context tree.

which is never a good sign.  ;)
Attachment #117789 - Attachment is obsolete: true
jkeiser, thoughts?  It looks like the style context for the mDisplayFrame in
nsComboboxControlFrame is just wrong.  Stepping through the code, the reresolve
for that context comes up with a reconstruct hint (!) so the context is left on
the frame... but the frame may never get reconstructed properly... (otherwise
I'd think it would have the right style context).

In any case, do you understand the mDisplayFrame code?  It seems to not call
Destroy() on frames, to not put mDisplayFrame in any child lists (I _did_ try
doing that, and that did not fix the crash), etc....:(
Created attachment 119384 [details] [diff] [review]
Proposed patch

Fix the block pseudo-frame inside the combobox to really be a pseudo-frame. 
This fixes the ReResolveStyleContext errors I was seeing.... (the blockframe
changes are removal of a hack that this bogosity necessitated, and a snuck-in
fix for some debug spew from the viewmanager).
Attachment #119316 - Attachment is obsolete: true
Attachment #119384 - Flags: superreview?(dbaron)
Attachment #119384 - Flags: review?(jkeiser)
Summary: Saving/Publishing page results in a crash → [FIX]Saving/Publishing page results in a crash
Comment on attachment 119384 [details] [diff] [review]
Proposed patch

mmm.  hack removal.  debug spew killer seems fine too.
Attachment #119384 - Flags: review?(jkeiser) → review+

Comment 37

15 years ago
bz, here's the bug I mentioned to you yesterday on IRC which sounded familiar
... bug 166596. What I remember seeing was that there was a stale reference to
some style by one of the components in the combobox, and the bug seemed timing
related, that is, a few strategically placed breakpoints would prevent the bug
from happening.

Does your proposed patch also fix this problem? The stack traces are slightly
As a note, the reason things break without this patch is that if the
:-moz-display-comboboxcontrol-frame gets a reconstruct hint we attempt to
reframe it, and hit the following code in nsCSSFrameConstructor::ContentRemoved:

      // For "select" add the pseudo frame after the last item is deleted
      nsIFrame* parentFrame = nsnull;
      if (parentFrame == selectFrame) {
        return NS_ERROR_FAILURE;

It _seems_ that we get a framechange hint because our -moz-user-input value is
changing from "auto" to "none" or back.  Not sure why that's happening,
exactly...  See discussion in bug 166596.

It's not clear to me why rods added that code, and it seems incredibly wrong to
me.... (xbl form controls, anyone?  ;) )
Comment on attachment 119384 [details] [diff] [review]
Proposed patch

If you're going to make the  function inline, please move it into the header. 
(Otherwise you have a pretty good chance of breaking some obscure compilers.)

sr=dbaron if you've tested this after roc landed.
Attachment #119384 - Flags: superreview?(dbaron) → superreview+
I have indeed.  After his landing we don't even have to save the page -- we
crash just opening it in Composer.  ;)  Fixed by this patch.

Checked in.
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 201551 has been marked as a duplicate of this bug. ***

Comment 42

15 years ago
Adding topcrash info for future reference...this was a recent topcrasher on the
Trunk from 4/9 - 4/12.
Keywords: testcase, topcrash+
Summary: [FIX]Saving/Publishing page results in a crash → [FIX]Saving/Publishing page results in a crash - Trunk [@ nsComboboxControlFrame::ReflowCombobox]

Comment 43

15 years ago
*** Bug 202025 has been marked as a duplicate of this bug. ***

Comment 44

15 years ago
Verified in the 2003-04-22-08 Macho and Win32 trunk builds.
Product: Browser → Seamonkey
Crash Signature: [@ nsComboboxControlFrame::ReflowCombobox]
You need to log in before you can comment on or make changes to this bug.