Bug 37902

Summary: REGRESSION(r57292): Safari/Win and Chromium/Win no longer pass the acid3 test.
Product: WebKit Reporter: Dimitri Glazkov (Google) <dglazkov>
Component: CSSAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, eric, hyatt, jamesr, mjs, webkit.review.bot
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 24300    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch darin: review+

Dimitri Glazkov (Google)
Reported 2010-04-20 16:39:21 PDT
The issue is in bit mis-alignment, caused by http://trac.webkit.org/browser/trunk/WebCore/rendering/style/RenderStyle.h?rev=57292#L204. Setting _insideLink to any value clobbers fields around it in weird and unanderstandable ways.
Attachments
Patch (1.40 KB, patch)
2010-04-21 09:24 PDT, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Patch (1.29 KB, patch)
2010-04-21 10:01 PDT, Dimitri Glazkov (Google) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1 2010-04-20 19:40:08 PDT
<rdar://problem/7887076>
Dimitri Glazkov (Google)
Comment 2 2010-04-20 19:41:37 PDT
will fix tomorrow, btw.
Maciej Stachowiak
Comment 3 2010-04-20 22:31:28 PDT
MSVC likes to make enum bitfields signed. _insideLink probably just needs to be bumped up to three bits. That or the type changed from EInsideLink to unsigned.
Dimitri Glazkov (Google)
Comment 4 2010-04-21 09:24:48 PDT
Created attachment 53962 [details] Patch
Darin Adler
Comment 5 2010-04-21 09:26:44 PDT
Comment on attachment 53962 [details] Patch The correct fix is not to make the bitfield larger. Instead we type the bitfield "unsigned" and add a comment with the type: unsigned _insideLink : 2; // EInsideLink See the line above. We might be able to change check-webkit-style to catch this.
Dimitri Glazkov (Google)
Comment 6 2010-04-21 10:01:12 PDT
Created attachment 53966 [details] Patch
Dimitri Glazkov (Google)
Comment 7 2010-04-21 10:02:08 PDT
Thanks Darin! I should've thought of this. dhyatt did all the casting for me already.
WebKit Review Bot
Comment 8 2010-04-21 10:04:20 PDT
Attachment 53966 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/rendering/style/RenderStyle.h:206: _insideLink is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dimitri Glazkov (Google)
Comment 9 2010-04-21 10:34:46 PDT
Landed as http://trac.webkit.org/changeset/57994.
WebKit Review Bot
Comment 10 2010-04-21 11:10:41 PDT
http://trac.webkit.org/changeset/57994 might have broken Qt Linux Release
Note You need to log in before you can comment on or make changes to this bug.