Bug 41371

Summary: REGRESSION (HTML Parser): CNN's Money site is not formatted correctly with AdBlock installed
Product: WebKit Reporter: Craig Post <sirgwain>
Component: DOMAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, commit-queue, eric, hsivonen
Priority: P1 Keywords: NeedsReduction, Regression
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
URL: http://money.cnn.com
Bug Depends on:    
Bug Blocks: 41115    
Attachments:
Description Flags
Screenshot of Webkit Nightly build view of CNN money
none
Screenshot of CNN Money in Safari 5
none
Patch none

Craig Post
Reported 2010-06-29 14:03:45 PDT
Created attachment 60048 [details] Screenshot of Webkit Nightly build view of CNN money Browsing to money.cnn.com shows some funky layout. Picture attached.
Attachments
Screenshot of Webkit Nightly build view of CNN money (266.22 KB, image/png)
2010-06-29 14:03 PDT, Craig Post 		no flags
Details
Screenshot of CNN Money in Safari 5 (453.61 KB, image/png)
2010-06-29 14:05 PDT, Craig Post 		no flags
Details
Patch (9.37 KB, patch)
2010-09-09 15:58 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Craig Post
Comment 1 2010-06-29 14:05:26 PDT
Created attachment 60049 [details] Screenshot of CNN Money in Safari 5
Alexey Proskuryakov
Comment 2 2010-07-01 13:06:56 PDT
I cannot reproduce this with r62241 nightly.
Craig Post
Comment 3 2010-07-01 14:51:00 PDT
(In reply to comment #2) > I cannot reproduce this with r62241 nightly. I just tested this out with Safari Version 5.0 (6533.16, r62241) and I'm still seeing it. Ah, I see the issue. I have the AdBlock extension installed. When I disable AdBlock it renders fine. However, it renders fine in base Safari 5 with AdBlock enabled.
Alexey Proskuryakov
Comment 4 2010-07-01 15:16:27 PDT
I can reproduce this with AdBlock 2.0.9 <http://safariadblock.com/> installed.
Alexey Proskuryakov
Comment 5 2010-07-01 15:53:09 PDT
This problem started with <http://trac.webkit.org/changeset/61285>, and only occurs with HTML5 parser enabled.
Eric Seidel (no email)
Comment 6 2010-07-01 16:10:01 PDT
Thank you for the report. The validator wasn't much help: http://html5.validator.nu/?doc=http://money.cnn.com/ obviously the site isn't valid HTML. The Safari extension is a xar archive, so "xar -x -f AdBlockForSafari.safariextz" will extract it. I suspect this is another <tag<foo> issue, but it's hard to know what the extension might be doing.
Adam Barth
Comment 7 2010-07-01 19:24:38 PDT
Eric, can you figure out the root cause here? Thanks.
Eric Seidel (no email)
Comment 8 2010-07-02 03:29:57 PDT
Happy to look. I won't get a chance to spend any time on this until wednesday when I'm back from vacation though.
Eric Seidel (no email)
Comment 9 2010-07-13 01:02:55 PDT
Probably the same as this mozilla issue: Having -- > terminate comments breaks CNN Money https://bugzilla.mozilla.org/show_bug.cgi?id=570309
Henri Sivonen
Comment 10 2010-09-08 04:40:01 PDT
http://www.w3.org/Bugs/Public/show_bug.cgi?id=10578
Adam Barth
Comment 11 2010-09-09 07:16:09 PDT
I'm going to try matching the Firefox behavior here.
Adam Barth
Comment 12 2010-09-09 15:58:15 PDT
Created attachment 67107 [details] Patch
Eric Seidel (no email)
Comment 13 2010-09-09 16:03:46 PDT
Comment on attachment 67107 [details] Patch LGTM. THanks.
WebKit Commit Bot
Comment 14 2010-09-10 14:18:31 PDT
Comment on attachment 67107 [details] Patch Rejecting patch 67107 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 20970 test cases. animations/combo-transform-translate+scale.html -> failed Exiting early after 1 failures. 99 tests run. 27.56s total testing time 98 test cases (98%) succeeded 1 test case (1%) had incorrect layout Full output: http://queues.webkit.org/results/3955353
Adam Barth
Comment 15 2010-09-10 14:20:27 PDT
> animations/combo-transform-translate+scale.html -> failed Seems unlikely.
Eric Seidel (no email)
Comment 16 2010-09-10 14:24:27 PDT
Comment on attachment 67107 [details] Patch Flaky test, I suspect.
Adam Barth
Comment 17 2010-09-10 14:25:25 PDT
Comment on attachment 67107 [details] Patch Clearing flags on attachment: 67107 Committed r67233: <http://trac.webkit.org/changeset/67233>
Adam Barth
Comment 18 2010-09-10 14:25:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.