342063 - Overlay well-formedness errors are reported twice

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Boris Zbarsky [:bzbarsky]]

First nsExpatDriver reports them, then the content sink does.  The latter report is not nearly as useful (since it's missing line/column info, etc) and should probably just be removed.

Comment 1

[Darin Fisher]

Not going to block 1.8.1 for this, but we'll happily consider a patch to fix this.

Comment 2

[Alex Vincent [:WeirdAl]]

I'm working on this now, as part of my work for bug 342164.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Alex, please write up a branch-safe version of this fix ASAP.  I don't think we want to ship this bug on branch, nor do I think we should block branch fixes on bug 342164.

Comment 4

[Alex Vincent [:WeirdAl]]

I'm on it.  Give me about two to three hours; I have to update my branch and do a couple compiles to get this.

Comment 5

[Alex Vincent [:WeirdAl]]

Attachment: broken overlay (testcase, part 1 of 2)

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: test file calling on overlay (testcase, part 2 of 2)

Comment 7

[Alex Vincent [:WeirdAl]]

Interesting aside:  When I load the base file locally (pointing to the broken overlay locally), the JS console gets flooded with an apparently endless cycle of these:

Error: mismatched tag. Expected: </overlay>.
Source File: file:///home/ajvincent/tests/brokenOverlay.xul
Line: 9, Column: 3
Source Code:
</window>--^

Comment 8

[Alex Vincent [:WeirdAl]]

(That's in Firefox 1.5.0.4 Debian)

Comment 9

[Alex Vincent [:WeirdAl]]

It took me a while to find the stack:

#0  nsConsoleService::LogMessage (this=0x31ec9e10, message=0x672200) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpcom/base/nsConsoleService.cpp:127
#1  0x057a83f2 in nsContentUtils::ReportToConsole (aFile=eXUL_PROPERTIES, aMessageName=0x5b0319c "MissingOverlay", aParams=0xbfffeedc, aParamsLength=1, aURI=0x34a26070, aSourceLine=@0x10e4ce0, aLineNumber=0, aColumnNumber=0, aErrorFlags=1, aCategory=0x5ac4110 "XUL Document") at /Users/ajvincent/mozilla.org/branch1_8/mozilla/content/base/src/nsContentUtils.cpp:2424
#2  0x058f88fa in nsXULDocument::ReportMissingOverlay (this=0x236ea00, aURI=0x364479e0) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/content/xul/document/src/nsXULDocument.cpp:3453
#3  0x059050b1 in nsXULDocument::ParserObserver::OnStopRequest (this=0x36447fe0, request=0x364487b0, aContext=0x0, aStatus=2147500034) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/content/xul/document/src/nsXULDocument.cpp:4658
#4  0x295591c7 in nsParser::OnStopRequest (this=0x36448540, request=0x364487b0, aContext=0x0, status=2147500034) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/parser/htmlparser/src/nsParser.cpp:2745
#5  0x03b97285 in nsStreamListenerTee::OnStopRequest (this=0x31eafe60, request=0x364487b0, context=0x0, status=2147500034) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/netwerk/base/src/nsStreamListenerTee.cpp:65
#6  0x03c14c2d in nsHttpChannel::OnStopRequest (this=0x36448780, request=0x364492f0, ctxt=0x0, status=2147500034) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:4120
#7  0x03b756a7 in nsInputStreamPump::OnStateStop (this=0x364492f0) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/netwerk/base/src/nsInputStreamPump.cpp:563
#8  0x03b757c5 in nsInputStreamPump::OnInputStreamReady (this=0x364492f0, stream=0x364490cc) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/netwerk/base/src/nsInputStreamPump.cpp:400
#9  0x010c5867 in nsInputStreamReadyEvent::EventHandler (plevent=0x36449394) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpcom/io/nsStreamUtils.cpp:120
#10 0x010690d9 in PL_HandleEvent (self=0x36449394) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpcom/threads/plevent.c:688
#11 0x01068f82 in PL_ProcessPendingEvents (self=0x610990) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpcom/threads/plevent.c:623
#12 0x010694a5 in _md_EventReceiverProc (info=0x610990) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpcom/threads/plevent.c:1559
#13 0x90823379 in CFRunLoopRunSpecific ()
#14 0x90822eb5 in CFRunLoopRunInMode ()
#15 0x92f02b90 in RunCurrentEventLoopInMode ()
#16 0x92f02297 in ReceiveNextEventCommon ()
#17 0x92f4a929 in _AcquireNextEvent ()
#18 0x92f4a774 in RunApplicationEventLoop ()
#19 0x2876373d in nsAppShell::Run (this=0x64a3f0) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/widget/src/mac/nsAppShell.cpp:93
#20 0x16ae7e70 in nsAppStartup::Run (this=0x64a3c0) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpfe/components/startup/src/nsAppStartup.cpp:207
#21 0x000062f0 in main1 (argc=3, argv=0xbffffa80, nativeApp=0x610680) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1249
#22 0x00006561 in main (argc=3, argv=0xbffffa80) at /Users/ajvincent/mozilla.org/branch1_8/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1750

Comment 10

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

So we were reporting a "missing" overlay when it wasn't really missing - just broken.

There's code elsewhere in nsXULDocument.cpp#2980 that correctly reports when an overlay is missing.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 231365 [details] [diff] [review]
patch, v1

Er... why are you changing this code?  The code to change is in nsXULContentSink and is the code touched by bug 335755 (which this is a regression from; see the dependencies).  I have no idea what the stack in comment 9 has to do with this bug.  All you have to do is remove the relevant block of code, then test.

Comment 12

[Alex Vincent [:WeirdAl]]

bz: that works for trunk, but not for branch.  Branch's XULContentSinkImpl doesn't have that patch applied to it.

You asked me for a branch patch... that's why it took me so long to debug this, because the branch code has a totally different route.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Er.... I thought bug 335755 got checked in on branch.  If it hasn't been, there is no problem there...

Comment 14

[Alex Vincent [:WeirdAl]]

Reference attachment 231368 [details], which was supposed to go here.

bz: This is a screenshot of the error console on 1.8 branch, with two error
messages.  As I understand this bug from comment 0, you want me to prevent the
second error message from appearing.  That's what my patch does.  Did I
misinterpret this bug?  Do I need to change the code on 1.8 branch somewhere
else?

Comment 15

[Boris Zbarsky [:bzbarsky]]

This bug is about the particular regression created by bug 335755.  Nothing more, nothing less.

If there are other bugs with overlays, please file them, of course.  ;)

Comment 16

[Alex Vincent [:WeirdAl]]

Attachment: screenshot (trunk)

Okay, bz:  which of these three error messages do you want me to get rid of?  (I hope you don't mean #1.)

Comment 17

[Boris Zbarsky [:bzbarsky]]

The second one.

Comment 18

[Alex Vincent [:WeirdAl]]

Fixed by checkin for bug 342164.