Bug 37902 - REGRESSION(r57292): Safari/Win and Chromium/Win no longer pass the acid3 test.
Summary: REGRESSION(r57292): Safari/Win and Chromium/Win no longer pass the acid3 test.
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P1 Normal
Assignee: Dimitri Glazkov (Google)
Keywords: InRadar
Depends on: 24300
  Show dependency treegraph
Reported: 2010-04-20 16:39 PDT by Dimitri Glazkov (Google)
Modified: 2010-04-21 11:10 PDT (History)
7 users (show)

See Also:

Patch (1.40 KB, patch)
2010-04-21 09:24 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Patch (1.29 KB, patch)
2010-04-21 10:01 PDT, Dimitri Glazkov (Google)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
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]
Comment 5 Darin Adler 2010-04-21 09:26:44 PDT
Comment on attachment 53962 [details]

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]
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