onload doesn't fire for XHTML documents that contain a script tag (Firefox's RSS preview is broken)

RESOLVED FIXED

Status

()

Core
DOM
--
blocker
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: tracy, Assigned: sicking)

Tracking

(5 keywords)

1.8 Branch
regression, smoketest, testcase, verified1.8.0.11, verified1.8.1.3
Points:
---
Bug Flags:
blocking1.8.1.3 +
blocking1.8.0.11 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
tested with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3pre) Gecko/20070308 BonEcho/2.0.0.3pre (also tested and failed on Linux and Mac builds nightly BE builds)

Steps to reproduce
1) Got to http://del.icio.us
2) Click on the RSS feed icon in the location bar
3) In the resulting Viewing Feed page, click on the Subscribe Now button

Tested results:
Nothing happens.  This page/subscribe dialog also looks incomplete. I'll attach a screen shot shortly

Expected results:
An add feed dialog appears which allows to add the feed to our bookmarks
Flags: blocking1.8.1.3?
(Reporter)

Comment 1

11 years ago
Created attachment 257825 [details]
screen shot of broken Viewing Feed page

Updated

11 years ago
Flags: blocking1.8.1.3? → blocking1.8.1.3+
(Reporter)

Comment 2

11 years ago
The regression window for this is between builds of
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-03-06-04-mozilla1.8 (working RSS subscription)
and ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2007-03-07-04-mozilla1.8 (broken RSS subscription) 
Flags: blocking1.8.1.3+ → blocking1.8.1.3?
seems to be a 1.8.1.3 branch regression only, because wfm on Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/2007030804 Minefield/3.0a3pre
Definitely a blocker until we resolve.
Flags: blocking1.8.1.3? → blocking1.8.1.3+
Backing out the patch for bug 371576 fixes this.
Blocks: 371576
Keywords: regression
Created attachment 257834 [details]
testcase

The presence of the script tag seems to prevent onload from firing.
Assignee: nobody → general
Component: RSS Discovery and Preview → DOM
Product: Firefox → Core
QA Contact: rss.preview → ian
Version: 2.0 Branch → 1.8 Branch
I can reproduce this on the 1.8.0 and 1.8 branches, but not on the trunk.
Summary: RSS feed Subscribe Now (Viewing Feed) doesn't subscribe to feed → onload doesn't fire for XHTML documents that contain a script tag (Firefox's RSS preview is broken)

Updated

11 years ago
Assignee: general → jonas
Flags: blocking1.8.0.11+

Updated

11 years ago
Flags: in-testsuite?
The problem is that in bug 371576 i made resuming the parser after it was blocked async. This was a bad idea for the branch.

What happens is this

<html>
  <head>
    <script src="data:text/plain,"/>
  </head>
  <body onload="..."/>
</html>

When we hit the <script> we fire off a new network load and block the parser. Then we sit and wait for that load to finish.

In the meantime the load for the document finishes and is removed from the loadgroup.

Once the network load for the script finishes we execute the script and tell the sink to resume parsing.

What used to happen:
We would on the current stack resume the parsing and once we hit the end of the document return.
We'd then unwind and remove the script-load from the loadgroup which would trigger onload.

What happens with the patch from bug 371576:
We'd fire an event to resume parsing asyncronously
We'd then unwind and remove the script-load from the loadgroup which would trigger onload.
We finally finish parsing off the event and at that point parse the <body> element and the onload attribute.


In other words. We still do fire onload. We just do it before we parse the body-onload attribute.

Patch coming up.
Created attachment 257890 [details] [diff] [review]
Patch to fix

This is simply a backout of the contentsink changes from bug 371576.
Attachment #257890 - Flags: superreview?(bzbarsky)
Attachment #257890 - Flags: review?(bzbarsky)
Attachment #257890 - Flags: approval1.8.1.3?
Attachment #257890 - Flags: approval1.8.0.11?
Comment on attachment 257890 [details] [diff] [review]
Patch to fix

r+sr=jst
Attachment #257890 - Flags: superreview?(bzbarsky)
Attachment #257890 - Flags: superreview+
Attachment #257890 - Flags: review?(bzbarsky)
Attachment #257890 - Flags: review+
Comment on attachment 257890 [details] [diff] [review]
Patch to fix

got r=preed over irl
Attachment #257890 - Flags: approval1.8.1.3?
Attachment #257890 - Flags: approval1.8.1.3+
Attachment #257890 - Flags: approval1.8.0.11?
Attachment #257890 - Flags: approval1.8.0.11+
that's a=preed
Keywords: fixed1.8.0.11, fixed1.8.1.3
Duplicate of this bug: 373316
This was checked in on branches yesterday. Keeping the bug open to investigate if trunk needs work too (though I don't think it does)
verified fixed for 1.8.1.3 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3pre) Gecko/2007030903 BonEcho/2.0.0.3pre

RSS Feed Preview and subscribe is now working fine. Also with Lvie Bookmarks and RSS Applications.
Keywords: fixed1.8.1.3 → verified1.8.1.3
also verified fixed for 2.0.0.3 RC1 Build identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.3) Gecko/2007030919 Firefox/2.0.0.3

RSS Feed Preview works fine

Comment 17

11 years ago
(In reply to comment #5)
> Backing out the patch for bug 371576 fixes this.
> 

Make sure we don't forget about bug 371321...
Yeah, i tested that bug before checking in.
since jonas is away, maybe jst can answer this question - the RSS feed functionality is different on the 1.8.0 branch (no feed preview UI). Testing with 1.5.0.10 I don't see any brokenness (I am able to add a feed by clicking on the RSS Icon in the URL bar), so I am trying to understand how the patch affects the 1.8.0 branch in terms of verification. Thanks.
The Firefox 1.5.0.x chrome doesn't include a way to test this bug. Testing the attached testcase should suffice for verifying the bug there, I think.
Keywords: testcase
The testcase passes fine in the 1.5.0.11 candidate build, but it does when I run it in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10) Gecko/20070216 Firefox/1.5.0.10 as well. If that is expected I can verify the bug.

(In reply to comment #20)
> The Firefox 1.5.0.x chrome doesn't include a way to test this bug. Testing the
> attached testcase should suffice for verifying the bug there, I think.
> 

(In reply to comment #21)
> The testcase passes fine in the 1.5.0.11 candidate build, but it does when I
> run it in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10)
> Gecko/20070216 Firefox/1.5.0.10 as well.

That 1.5.0.10 build doesn't include the patch for bug 371576 (which caused this bug), so the fact that it passes is expected. To test a failing build, you need a 1.8.0 branch build from 2007-03-07. This bug was only on the branches for ~1 day, it wasn't a regression from 1.5.0.10.
verified on the 1.8.0 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.11) Gecko/20070312 Firefox/1.5.0.11. I used Gavin's testcase in Comment 6 to verify the bug. I also checked back and did see the breakage on the 20070307 build, and can verify that the test case does fail on that build. Thanks for the clarification, I am adding the verified keyword to this bug.

Keywords: fixed1.8.0.11 → verified1.8.0.11
Should this bug not become resolved->fixed?
I'd like to check that this for sure doesn't break on trunk first. Just testing seems to indicate that it works, but I'd like to make sure. If someone could write up some mochikit tests that would help a lot.
Created attachment 261795 [details] [diff] [review]
add mochikit test

Adds a MochiKit test for this bug.  I ran the test on trunk (pass), latest 1.8 branch (pass), and a build from the regression window (fail).
Attachment #261795 - Flags: review?(jonas)
Attachment #261795 - Flags: review?(jonas) → review+
Checked in the test.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
http://lxr.mozilla.org/mozilla/source/content/base/test/test_bug373181.xhtml
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.