Bug 90376 - [Qt][WK2] fast/viewport/viewport-91.html still fails after r121555 and r121661
Summary: [Qt][WK2] fast/viewport/viewport-91.html still fails after r121555 and r121661
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Balazs Kelemen
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 79666 90286
  Show dependency treegraph
 
Reported: 2012-07-02 05:39 PDT by János Badics
Modified: 2012-07-03 01:22 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.41 KB, patch)
2012-07-02 07:25 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-06 (339.63 KB, application/zip)
2012-07-02 09:01 PDT, WebKit Review Bot
no flags Details
Patch (5.24 KB, patch)
2012-07-02 09:16 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description János Badics 2012-07-02 05:39:07 PDT
This test:
fast/viewport/viewport-91.html fails on qt-5.0-wk2 after r121555 and r121661

See original bug at
https://bugs.webkit.org/show_bug.cgi?id=88114
Comment 1 Csaba Osztrogonác 2012-07-02 05:42:32 PDT
https://trac.webkit.org/changeset/121661 fixed it in general, but it didn't fix it on Qt-WK2.
Comment 2 Balazs Kelemen 2012-07-02 06:39:40 PDT
I think the WebKit2 part of the original patch is not enough:

diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
-    ViewportAttributes attr = computeViewportAttributes(m_page->viewportArguments(), minimumLayoutFallbackWidth, deviceWidth, deviceHeight, static_cast<int>(160 * m_page->deviceScaleFactor()), m_viewportSize);
+    ViewportAttributes attr = computeViewportAttributes(m_page->viewportArguments(), minimumLayoutFallbackWidth, deviceWidth, deviceHeight, m_page->deviceScaleFactor(), m_viewportSize);

Only the Mac port set up the deviceScaleFactor for the page, other ports leave it to the embedder. I think we should either set this up for every port or just allow ports to use the old (deprecated?) behavior.
Comment 3 Balazs Kelemen 2012-07-02 07:01:45 PDT
(In reply to comment #2)
> I think the WebKit2 part of the original patch is not enough:
> 
> diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
> -    ViewportAttributes attr = computeViewportAttributes(m_page->viewportArguments(), minimumLayoutFallbackWidth, deviceWidth, deviceHeight, static_cast<int>(160 * m_page->deviceScaleFactor()), m_viewportSize);
> +    ViewportAttributes attr = computeViewportAttributes(m_page->viewportArguments(), minimumLayoutFallbackWidth, deviceWidth, deviceHeight, m_page->deviceScaleFactor(), m_viewportSize);
> 
> Only the Mac port set up the deviceScaleFactor for the page, other ports leave it to the embedder. I think we should either set this up for every port or just allow ports to use the old (deprecated?) behavior.

Ehh, I'm wrong, this should not change anything. But than how was it good, and how went it wrong???
Comment 4 Balazs Kelemen 2012-07-02 07:25:26 PDT
Created attachment 150419 [details]
Patch
Comment 5 Balazs Kelemen 2012-07-02 07:27:36 PDT
(In reply to comment #4)
> Created an attachment (id=150419) [details]
> Patch

We need to use the constant for layoutTestController.dumpConfigurationForViewport because the device dpi value has no chance to go through the UI process that calculates the deviceScaleFactor value.
Comment 6 Build Bot 2012-07-02 08:32:23 PDT
Comment on attachment 150419 [details]
Patch

Attachment 150419 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13124196
Comment 7 Csaba Osztrogonác 2012-07-02 08:33:25 PDT
Skipped on qt-5.0-wk2 by r121680. Please unskip it with the proper fix.
Comment 8 WebKit Review Bot 2012-07-02 09:00:59 PDT
Comment on attachment 150419 [details]
Patch

Attachment 150419 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13131184

New failing tests:
fast/loader/loadInProgress.html
Comment 9 WebKit Review Bot 2012-07-02 09:01:03 PDT
Created attachment 150433 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 10 Balazs Kelemen 2012-07-02 09:16:29 PDT
Created attachment 150435 [details]
Patch
Comment 11 Csaba Osztrogonác 2012-07-03 01:18:55 PDT
Comment on attachment 150435 [details]
Patch

rs=me
Comment 12 Balazs Kelemen 2012-07-03 01:22:44 PDT
Comment on attachment 150435 [details]
Patch

Clearing flags on attachment: 150435

Committed r121740: <http://trac.webkit.org/changeset/121740>
Comment 13 Balazs Kelemen 2012-07-03 01:22:52 PDT
All reviewed patches have been landed.  Closing bug.