Bug 143479

Summary: REGRESSION: Wrong layout in http://www.heroquestclassic.com/heroes-fe-de-erratas-y-un-breve-descanso/
Product: WebKit Reporter: Andres Gomez Garcia <agomez>
Component: EvangelismAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, benjamin, cgarcia, darin, koivisto, rniwa, sam, simon.fraser, timothy, webkit-bug-importer, yoon
Priority: P2 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
reduced test case none

Description Andres Gomez Garcia 2015-04-07 05:42:33 PDT
WebKitGtk 2.8.0

Open http://www.heroquestclassic.com/heroes-fe-de-erratas-y-un-breve-descanso/ in MiniBrowser

The layout is completely wrong, with all the content in a narrow column at the left of the page.

This is also reproducible with Epiphany 3.16.0 running also WebKitGtk 2.8.0
Comment 1 Carlos Garcia Campos 2015-04-09 00:16:24 PDT
It works with WebKitGTK+ 2.6.x, but fails with trunk/2.8. The problem is also reproducible in mac, so this looks like a recent regression in WebCore.
Comment 2 Darin Adler 2015-04-09 22:47:07 PDT
Next step would be to reduce this to make a much smaller test case showing the problem.
Comment 3 Carlos Garcia Campos 2015-04-10 00:19:58 PDT
Knowing the revision that introduced the regression would also help. I started to bisect yesterday, but all the build breaks made it impossible to do a bisect using the GTK+ port :-(
Comment 4 Gwang Yoon Hwang 2015-04-10 00:22:45 PDT
In Safari Version 8.0.5 (10600.5.17), it seems okay, but
I don't know which revision it uses.
Comment 5 Darin Adler 2015-04-10 00:27:32 PDT
Note that we need the reduction anyway to help us make a regression test, even if we don't figure out exactly what version this regressed in.
Comment 6 Gwang Yoon Hwang 2015-04-10 00:40:05 PDT
(In reply to comment #5)
> Note that we need the reduction anyway to help us make a regression test,
> even if we don't figure out exactly what version this regressed in.

I agree to make a regression test for this bug.
If there is nobody have a time to do it, I'll do it.
Comment 7 Radar WebKit Bug Importer 2015-04-11 21:45:29 PDT
<rdar://problem/20511704>
Comment 8 Darin Adler 2015-04-11 22:27:22 PDT
Created attachment 250595 [details]
reduced test case

It’s a CSS rule problem. I attached a simple test case.
Comment 9 Darin Adler 2015-04-11 22:29:47 PDT
Maybe r182321? The new Bloom filter?
Comment 10 Darin Adler 2015-04-11 22:35:49 PDT
Or some earlier change to the CSS rule system.
Comment 11 Darin Adler 2015-04-12 09:54:37 PDT
Looks like this isn’t a bug at all. The website was depending on a bug in an older version of WebKit. The :not expression should match almost everything, and it does.
Comment 12 Darin Adler 2015-04-12 09:57:47 PDT
The style sheet named pb-view.css has these rules in it:

.template-wrapper .span3, :not(.portfolio-grid li.span3) {
    width: 22.75%;
}

.template-wrapper .span12 .span3, :not(.portfolio-grid li.span3) {
    width: 21.25%;
}

Those rules are going to apply to practically every element on the page, and are going to set their width to about a fifth of what the should be. Older versions of WebKit without proper :not support would incorrectly ignore these rules. I believe the error here was using a comma rather than using a space. I believe the :not is supposed to be a descendant selector.

I suggest we not change WebKit and instead get the website to fix their broken style sheet.
Comment 13 Darin Adler 2015-04-12 09:58:59 PDT
I’d like to understand why this problem doesn’t happen with non-WebKit web browsers.
Comment 14 Darin Adler 2015-04-12 09:59:58 PDT
Note that I say these rules set the widths of elements to about a fifth what they should be. But this happens over and over again, since these rules apply to almost every element on the page, so they apply to the body element, and each div element, and the img element, and so on. So the widths get smaller and smaller (1/5 of 1/5 of 1/5).
Comment 15 Benjamin Poulain 2015-04-12 12:41:04 PDT
(In reply to comment #13)
> I’d like to understand why this problem doesn’t happen with non-WebKit web
> browsers.

It is because of our support for CSS 4.

The other browsers only support CSS Selectors Level 3. In Level 3, it was only valid to use a simple selector inside :not().

In WebKit, we support almost full selector lists (:visited and :link do not match). The rule ":not(.portfolio-grid li.span3)" would be invalid for older browsers. For us it is equivalent to ":not(.portfolio-grid >> li.span3)" and is completely valid.
Comment 16 Darin Adler 2015-04-12 13:06:45 PDT
So this was skipped in older browsers because it was not valid. But in recent WebKit it matches everything. I think this is going to have to be fixed by the website owners. Unless we find this kind of mistake is common and we have to come up with some kind of compatibility quirk.
Comment 17 Antti Koivisto 2015-04-17 01:05:55 PDT
Another way to think about this is that CSS 4 :not may not be web compatible as specified.
Comment 18 Andres Gomez Garcia 2015-04-24 01:47:20 PDT
Thanks guys! I will report back to the website.
Comment 19 Timothy Hatcher 2015-04-24 10:43:02 PDT
Jon Davis reached out to the site owner on 4/16. The issue comes from a WordPress plugin called Aqua Page Builder. Jon contacted the plugin author on 4/20, who has mentioned he is no longer actively developing the plugin, but said he was open to recommendations. We sent over the recommended changes and waiting to hear back on whether he will implement them or not.
Comment 20 Benjamin Poulain 2015-04-24 13:46:55 PDT
Can someone try a pull request? https://github.com/syamilmj/Aqua-Page-Builder/pulls
Comment 21 Andres Gomez Garcia 2015-04-24 13:47:58 PDT
Oh, great!

Well, I sent a comment back to the site but it seems you have already done much more work on that direction.

Thanks a lot!