Summary: | [Qt] rendering error in mediawiki | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrian von Bidder <avbidder> | ||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | CLOSED FIXED | ||||||||||||
Severity: | Normal | CC: | abecsi, adawit, ap, commit-queue, hausmann, kenneth, luiz, ossy, tonikitoo | ||||||||||
Priority: | P3 | Keywords: | Qt, QtTriaged | ||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | Linux | ||||||||||||
URL: | http://www.debianclusters.org/index.php/Main_Page | ||||||||||||
Attachments: |
|
Description
Adrian von Bidder
2010-06-02 11:07:07 PDT
FYI. This reproducible using QtTestBrowser in QtWebKit 2.0 as well... 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 can reproduce it with QtTestBrowser (WebKit trunk r60630) (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. 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 (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. 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? Is this setting set for Qt? document()->settings()->needsSiteSpecificQuirks() I cc-ed Andras, he is our CSS parser guru. Have you got any idea? (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. Yes, comment 8 sounds like the best explanation. Created attachment 58162 [details]
Patch
Here is a patch that fixes the problem.
(In reply to comment #8) > Is this setting set for Qt? > > document()->settings()->needsSiteSpecificQuirks() Nope, it is not... See patch. 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.
Created attachment 58165 [details] Patch - rev. #2 Fixed the attribute documentation based on review in comment #14... Comment on attachment 58165 [details]
Patch - rev. #2
r=me, as we can still change the name etc before cherry-picking this change.
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. 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. (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- 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. 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.
Created attachment 58541 [details]
Patch - rev. #3
Renamed attribute to 'SiteSpecificQuirksEnabled' as requested...
(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. Can you supply a version that will? Created attachment 58553 [details]
Same patch vs trunk...
Patch against the current trunk version of webkit...
Comment on attachment 58553 [details] Same patch vs trunk... Clearing flags on attachment: 58553 Committed r61064: <http://trac.webkit.org/changeset/61064> 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? (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... (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? (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. (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... Site authors of sites currently affected by quirks who want to make their site standard compliant. Makes sense, thanks :) I'll swap the last two enums in qwebsettings.h to ensure binary compatibility when including it in the release branch. Revision r61064 cherry-picked into qtwebkit-2.0 with commit 815815940c24badaefd9b4deeb1851534bd677e0 http://trac.webkit.org/changeset/61251 corrects the enum order |