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+

Description Dimitri Glazkov (Google) 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.
Comment 1 Mark Rowe (bdash) 2010-04-20 19:40:08 PDT
<rdar://problem/7887076>
Comment 2 Dimitri Glazkov (Google) 2010-04-20 19:41:37 PDT
will fix tomorrow, btw.
Comment 3 Maciej Stachowiak 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.
Comment 4 Dimitri Glazkov (Google) 2010-04-21 09:24:48 PDT
Created attachment 53962 [details]
Patch
Comment 5 Darin Adler 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.
Comment 6 Dimitri Glazkov (Google) 2010-04-21 10:01:12 PDT
Created attachment 53966 [details]
Patch
Comment 7 Dimitri Glazkov (Google) 2010-04-21 10:02:08 PDT
Thanks Darin! I should've thought of this. dhyatt did all the casting for me already.
Comment 8 WebKit Review Bot 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.
Comment 9 Dimitri Glazkov (Google) 2010-04-21 10:34:46 PDT
Landed as http://trac.webkit.org/changeset/57994.
Comment 10 WebKit Review Bot 2010-04-21 11:10:41 PDT
http://trac.webkit.org/changeset/57994 might have broken Qt Linux Release