214874 - [FIXr]scripts are executed when innerHTML is set

Status: RESOLVED FIXED

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

Description

[Stef van der Made]

1 Open the page www.tweakers.net
2 The page starts loading OK 
3 The page starts opening webads and turns into a white page

Comment 1

[Bill Mason]

WFM 20030729 PC/WinXP.

You have given no information about what version of Mozilla you are using.
Nor is there any reason cited for assigning this to JavaScript Engine.

Resolving invalid; insufficient information in the report.
Please reopen if you can provide a more complete bug report.
http://www.mozilla.org/quality/bug-writing-guidelines.html

Comment 2

[Stef van der Made]

The version of mozilla that I'm using is 20030802 on Linux and Windows 2000 SP2.

The reason to assign this bug to javascript is this source of the page

<script language='javascript'
src='http://adserver.webads.nl/jserver/ACC_RANDOM=1059854227/SITE=TWEAKERS/AREA=TWEAKERS_HOME/AAMSZ=TEXTBOX/WPC=YES/POSITION=TOP'></script><img
src='http://www.tweakers.net/advertenties/adlog.php?bannerid=141&amp;clientid=83&amp;zoneid=18&amp;source=&amp;block=0&amp;capping=0&amp;cb=5d7063a2eb906c0691edf9d2ff4bf40f'
width='0' height='0' alt='' style='width: 0px; height: 0px;'><a
HREF="http://adserver.webads.nl/adclick/CID=fffffffcfffffffcfffffffc/ACC_RANDOM=1059854227/SITE=TWEAKERS/AREA=TWEAKERS_HOME/AAMSZ=TEXTBOX/WPC=YES/POSITION=TOP"
TARGET="_NEW"><img SRC="http://images.webads.nl/ads/default.gif" ALT="" 
width=-1 height=-1 BORDER=0></a>

Comment 3

[Olivier Cahagne]

Attachment: Screenshot of redirect

I also get redirected to this page (sometimes blank, sometimes with a real ad
content). The <title> is strange: textboxtest.

Comment 4

[Andrew Schultz]

Attachment: external script (load the next one)

Comment 5

[Andrew Schultz]

Attachment: testcase

load this one.
with linux trunk 20030802, it just shows "bar"

Comment 6

[Andrew Schultz]

this regressed between linux trunk 2003073109 and 2003080105.
Axel: could this have regressed due to your checkin for bug 151002?

certainly now JS Engine
==> DOM 0

Comment 7

[Andrew Schultz]

oops.
==> DOM 0 for real

Axel: could this have regressed due to bug 151002

Comment 8

[Axel Hecht]

bug 151002 has nothing to do with anything in the vicinity of the testcase.
no idea what's going on, though. bzbarsky changed something with scripts in what I
think is the time between wfm and the buggy behaviour. Though I wouldn't know
why this would make a difference. Of course, there's deDOMification, too.
bz, mind taking a look?

(btw, I can reproduce on my debug build from 08/01/2003 on WinME.)
bonsai query for the time:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fdom+mozilla%2Fcontent&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=07%2F28%2F2003&maxdate=08%2F02%2F2003&cvsroot=%2Fcvsroot

Comment 9

[Arjen Anker]

Having the same problems with www.tweakers.net and www.cdfreaks.com.

Build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030801
Mozilla Firebird/0.6.1

I tracked it down to those
document.getElementById('foo').innerHTML=document.getElementById('bar').innerHTML;
things too. Saving the page and commenting that out in the scripts corrects te
problem, but (ofcourse) it keeps the ads from appearing.

Comment 10

[Anton van Bohemen]

The testcase in Comment #7 definately shows the problem. However, I cannot
replicate the bug every time. For example, when I hit Stop somewhere halfway
loading www.tweakers.net, I can reload it without any problem and the banner
also shows up normally.

Comment 11

[Andrew Schultz]

> bonsai query for the time

the regression is between trunk 2003073005 and 2003073105 (not between 7/31 and
8/01 as I said earlier), so this is definitely 214081

Comment 12

[Bram van Leur]

Problem remaining in 2003080704, since trunk is frozen will this be blocking 1.5
final (or beta)? 
(it probably affects many more sites than only tweakers.net and cdfreaks.com)

Comment 13

[Andrew Schultz]

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

Comment 14

[Andrew Schultz]

resummarizing... please fix if I don't know what's going on.

this does seem like a significant regression, but it hasn't been reported for
other sites (that I know of)

Comment 15

[Arjen Anker]

I don't believe it's just document.write() and document.close(). I think they
work fine, if not used with thos innerHTML things.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Actually, the regression here is that now scripts get executed when innerHTML is
set.  This is definitely a regression from bug 214081; taking.

Fwiw, the original fix that I regressed was that for bug 116834.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

This basically backs out the scriptloader changes from bug 214081 (except it
leaves in the mDocument->IsScriptEnabled() check, which we want to treat the
same way as scripts being disabled in the script context).

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 129510 [details] [diff] [review]
Patch

jst, could you please review this?

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 129510 [details] [diff] [review]
Patch

r+sr=jst. If anything, I'd combine the early returns and the calls to
FireScriptAvailable() into one place, but either way works for me...

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 129510 [details] [diff] [review]
Patch

Could this please be approved for 1.5b?  Fixes a pretty serious innerHTML
regression.

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 129510 [details] [diff] [review]
Patch

a=dveditz for drivers

Comment 22

[Boris Zbarsky [:bzbarsky]]

Fix checked in to 1.5b trunk.

Comment 23

[Stef van der Made]

The bug fix has also been implemented now in the latest nightly build. Thanks