Bug 40073 – [Qt] rendering error in mediawiki

• Component: Layout and Rendering
• Status: CLOSED FIXED

[Adrian von Bidder]

See KDE: https://bugs.kde.org/show_bug.cgi?id=240465 and the attached screenshot.

happens with many mediawiki sites, not only debianclusters.  khtml and other browsers work fine; webkit doesn't have this problem with wikipedia or some other mediawiki sites, so it seems to be a particular version of their template that causes the bug.

Browser: kpart-webkit 0.9svn1129464-1 (Debian version, it's the current unstable package update to a newer svn snapshot) and libqt4-webkit 4:4.6.2-5.

cheers
-- vbi

[Dawit A.]

FYI. This reproducible using QtTestBrowser in QtWebKit 2.0 as well...

[Alexey Proskuryakov]

I cannot reproduce this with ToT WebKit, so I suspect that Qt has an old version of WebKit. Issues like this have been addressed in bug 28350, bug 29792, and bug 35507.

[Csaba Osztrogonác]

I can reproduce it with QtTestBrowser (WebKit trunk r60630)

[Csaba Osztrogonác]

(In reply to comment #3)
> I can reproduce it with QtTestBrowser (WebKit trunk r60630)
It is the ToT of WebKit and I tried with Qt-4.6.0 and Qt-4.7 trunk.

[Kenneth Rohde Christiansen]

Luiz, can you check this with pure Mac and Qt Mac build? Would be nice to know if this is a Qt only issue or not

[Csaba Osztrogonác]

(In reply to comment #5)
> Luiz, can you check this with pure Mac and Qt Mac build? Would be nice to know if this is a Qt only issue or not

I tried with an old Safari on Windows and works as we expect.

[Kenneth Rohde Christiansen]

as far as I remember, the mediawiki pages import illegal css when the user agent contain "KHTML" and we (WebKit community) had to make a quirk. Could it be that this quirk is not enabled for Qt?

[Kenneth Rohde Christiansen]

Is this setting set for Qt?

document()->settings()->needsSiteSpecificQuirks()

[Csaba Osztrogonác]

I cc-ed Andras, he is our CSS parser guru. Have you got any idea?

[Dawit A.]

(In reply to comment #2)
> I cannot reproduce this with ToT WebKit, so I suspect that Qt has an old version of WebKit. Issues like this have been addressed in bug 28350, bug 29792, and bug 35507.

I checked and the fixes from all of those reports are there in QtWebKit 2.0 ; so something else is causing or preventing those fixes from having any effect in QtWebKit... Perhaps the suggestion in comment #8 ?? Dunno.

[Alexey Proskuryakov]

Yes, comment 8 sounds like the best explanation.

[Dawit A.]

Created attachment 58162 [details]
Patch

Here is a patch that fixes the problem.

[Dawit A.]

(In reply to comment #8)
> Is this setting set for Qt?
> 
> document()->settings()->needsSiteSpecificQuirks()

Nope, it is not... See patch.

[Kenneth Rohde Christiansen]

Comment on attachment 58162 [details]
Patch

WebKit/qt/Api/qwebsettings.cpp:434
 +          sites that use certain frameworks, such as the older versions of MediaWiki, may not render properly.
I'm not sure if we really want references to MediaWiki here. Also, I do not like "not render or display properly in it", as the case is actually that is is a workaround for broken sites, not just sites that do not look good.

WebKit/qt/Api/qwebsettings.h:77
 +          SiteSpecificQuirksModeEnabled
We might want a more understanding setting, Let's see what Simon says.

Apart from that, the idea is fine. R- due to semi-optimal documentation.

[Dawit A.]

Created attachment 58165 [details]
Patch - rev. #2

Fixed the attribute documentation based on review in comment #14...

[Kenneth Rohde Christiansen]

Comment on attachment 58165 [details]
Patch - rev. #2

r=me, as we can still change the name etc before cherry-picking this change.

[Antonio Gomes]

no commit message? should be the same as in ChangeLog.

> diff --git a/WebKit/qt/Api/qwebsettings.cpp b/WebKit/qt/Api/qwebsettings.cpp
> index a5fc794..9631f05 100644
> --- a/WebKit/qt/Api/qwebsettings.cpp
> +++ b/WebKit/qt/Api/qwebsettings.cpp
> +    \value SiteSpecificQuirksModeEnabled This setting enables WebKit's workaround for broken sites. It is 
> +        enabled by default.

I do not think it is about WebKit workarounds. CSS Quirk Mode is a way that browsers (not only WebKit) support the old and broken CSS use that works for IE6 and family.

http://www.quirksmode.org/css/quirksmode.html

You are not mentioning about CSS in the setting name. Should you? or at least in the documentation. It is a CSS Mode and has Quirk and Strict modes.

I would not commit it as is :-( due to the documentation and the setting name.

[Kenneth Rohde Christiansen]

Safari has it enabled by default and there is a menu for disabling it

Safari->Develop->Disable Site-Specific Hacks

Antonio, this has nothing to do with CSS Quirks and Strict mode, but are work around for broken sites.

[Kenneth Rohde Christiansen]

(In reply to comment #18)
> Safari has it enabled by default and there is a menu for disabling it
> 
> Safari->Develop->Disable Site-Specific Hacks
> 
> Antonio, this has nothing to do with CSS Quirks and Strict mode, but are work around for broken sites.

I guess the QuicksMode in the name is confusing and maybe we should go for SiteSpecificHacksEnabled instead ? Let me cq-

[Alexey Proskuryakov]

The reason to have a way to disable these workarounds is to give developers of those specific sites a way to test how their sites would work without the workarounds. Users never need to disable these workarounds.

[Kenneth Rohde Christiansen]

Comment on attachment 58165 [details]
Patch - rev. #2

Can you supply a new patch where this is renamed QWebSettings:SiteSpecificQuirksEnabled due remove the Mode; then I will re-review.

[Dawit A.]

Created attachment 58541 [details]
Patch - rev. #3

Renamed attribute to 'SiteSpecificQuirksEnabled' as requested...

[Dawit A.]

(In reply to comment #21)
> (From update of attachment 58165 [details])
> Can you supply a new patch where this is renamed QWebSettings:SiteSpecificQuirksEnabled due remove the Mode; then I will re-review.

Done... Note that this version will not cleanly apply to the trunk version of qtwebkit since a new WebGLEnabled option is present there.

[Kenneth Rohde Christiansen]

Can you supply a version that will?

[Dawit A.]

Created attachment 58553 [details]
Same patch vs trunk...

Patch against the current trunk version of webkit...

[WebKit Commit Bot]

Comment on attachment 58553 [details]
Same patch vs trunk...

Clearing flags on attachment: 58553

Committed r61064: <http://trac.webkit.org/changeset/61064>

[Simon Hausmann]

Dawit, are you saying that in the KDE webkit integration you need to disable these site specific quirks, because of the user agent that you're sending?

[Dawit A.]

(In reply to comment #27)
> Dawit, are you saying that in the KDE webkit integration you need to disable these site specific quirks, because of the user agent that you're sending?

Nope. This is not a KDE webkit specific issue. It is a QtWebKit bug that affects all QtWebKit based browser. See comments #1-3 above...

[Simon Hausmann]

(In reply to comment #28)
> (In reply to comment #27)
> > Dawit, are you saying that in the KDE webkit integration you need to disable these site specific quirks, because of the user agent that you're sending?
> 
> Nope. This is not a KDE webkit specific issue. It is a QtWebKit bug that affects all QtWebKit based browser. See comments #1-3 above...

Right, but what I'd like to find out _who_ is going to use this web setting and disable the site specific quirks?

[Antonio Gomes]

(In reply to comment #29)
> (In reply to comment #28)
> > (In reply to comment #27)
> > > Dawit, are you saying that in the KDE webkit integration you need to disable these site specific quirks, because of the user agent that you're sending?
> > 
> > Nope. This is not a KDE webkit specific issue. It is a QtWebKit bug that affects all QtWebKit based browser. See comments #1-3 above...
> 
> Right, but what I'd like to find out _who_ is going to use this web setting and disable the site specific quirks?

that is a good question. why to disable it at all or why its value in Settings.cpp|h is not true by default already?

fwiw, mac, win, gtk and chromium ports also have this setting as a public api.

[Dawit A.]

(In reply to comment #29)
> (In reply to comment #28)
> > (In reply to comment #27)
> > > Dawit, are you saying that in the KDE webkit integration you need to disable these site specific quirks, because of the user agent that you're sending?
> > 
> > Nope. This is not a KDE webkit specific issue. It is a QtWebKit bug that affects all QtWebKit based browser. See comments #1-3 above...
> 
> Right, but what I'd like to find out _who_ is going to use this web setting and disable the site specific quirks?

Developers. The patch is implemented that way because other browsers provide a means of enabling/disabling this feature. See comment #18 and comment #20. It is only applicable to developers...

[Kenneth Rohde Christiansen]

Site authors of sites currently affected by quirks who want to make their site standard compliant.

[Simon Hausmann]

Makes sense, thanks :)

I'll swap the last two enums in qwebsettings.h to ensure binary compatibility when including it in the release branch.

[Simon Hausmann]

Revision r61064 cherry-picked into qtwebkit-2.0 with commit 815815940c24badaefd9b4deeb1851534bd677e0

[Simon Hausmann]

http://trac.webkit.org/changeset/61251 corrects the enum order