Bug 56669 – [Qt] QtWebKit rendering problem when maximizing and doing a back

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Aparna Nandyal]

Preconditions - QtTestBrowser is not maximized
Steps to reproduce:
1. From QtTestBrowser, go to google.com 
2. Search for example meego - Page 1
3. Open a link, say first link from search page - Page 2
4. Maximize the window
5. Now press back button

Expected - Page 1 content should now be displayed in entire browser

Actual - There is an overlap of page 1 and page 2 content

[Benjamin Poulain]

Setting P1 since it is a regression.

For reference: https://bugs.webkit.org/show_bug.cgi?id=51892

[Aparna Nandyal]

Created attachment 86361 [details]
Patch for review

Bug is caused due to changes made in http://trac.webkit.org/changeset/79167

On resizing and going back, contentsResized() is not getting called. Before the above changeset, contentsResized was called from ScrollView::setFrameRect().
Now ScrollView::setBoundsSize makes the call to contentsResized. Changes made are similar to changes made in resize() of Widget.h.

[Alexis Menard (darktears)]

Comment on attachment 86361 [details]
Patch for review

Look goods to me. It was related to this changes too : http://trac.webkit.org/changeset/79450

[Alexis Menard (darktears)]

Perhaps you should put my name for the test just in case we have stability problems, so if needed it's possible to find the original author :).

[Aparna Nandyal]

(In reply to comment #4)
> Perhaps you should put my name for the test just in case we have stability problems, so if needed it's possible to find the original author :).

I just left a message to you IRC, I was not sure how to add your name too as author :). Where should it go?

[Aparna Nandyal]

Created attachment 86367 [details]
Patch v02 for review

Adding Alexis as author of the auto test.

[Antonio Gomes]

Comment on attachment 86367 [details]
Patch v02 for review

View in context: https://bugs.webkit.org/attachment.cgi?id=86367&action=review

Was it wrong in the original patch? no layout test catching it? Is it possible to test it through DRT?

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:442
> +void tst_QWebView::rendering()

this name is meaningless.

[Benjamin Poulain]

Comment on attachment 86367 [details]
Patch v02 for review

View in context: https://bugs.webkit.org/attachment.cgi?id=86367&action=review

No idea about the validity of change itself though, maybe Andreas... :)

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:472
> +    QApplication::processEvents();

Can't you just waitForSignal() on loadFinished()?

[Andreas Kling]

Comment on attachment 86367 [details]
Patch v02 for review

View in context: https://bugs.webkit.org/attachment.cgi?id=86367&action=review

> Source/WebCore/loader/FrameLoader.cpp:2196
> +        view->setBoundsSize(IntSize(rect.width(), rect.height()));

view->setBoundsSize(rect.size());

[Aparna Nandyal]

Created attachment 86582 [details]
Patch v03 with comments implemented

1. Renamed the function in test. 
2. Corrected the code as per comments from Andreas.
3. Corrected the code in test
3. Could not add layout test (DRT) as window.resizeTo(w, h) is not happening in QtTestBrowser.

[Alexis Menard (darktears)]

Comment on attachment 86582 [details]
Patch v03 with comments implemented

LGTM :D

[Andreas Kling]

Comment on attachment 86582 [details]
Patch v03 with comments implemented

LGTM, too :)

[WebKit Commit Bot]

Comment on attachment 86582 [details]
Patch v03 with comments implemented

Clearing flags on attachment: 86582

Committed r81775: <http://trac.webkit.org/changeset/81775>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/81775 might have broken GTK Linux 64-bit Debug

[Laszlo Gombos]

Reopen as this test fails on Symbian and possible on other "small screen" platforms - (see http://build.webkit.sed.hu/builders/Qt%20Symbian%20ARMv5%20Release/builds/1156/steps/API%20tests/logs/stdio)

FAIL!  : tst_QWebView::renderingAfterMaxAndBack() Compared values are not the same
   Loc: [D:/w/qt-symbian-release/build/Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp(504)]

Perhaps the test should render smaller sized content - e.g. 50x50 instead of 1024x768. Disabling the scrollbars would be a good ideas as well (see http://trac.webkit.org/changeset/84332)

CCd Yi who might be able to help with testing on Symbian.

[Joel Parks]

(In reply to comment #16)
> Reopen as this test fails on Symbian and possible on other "small screen" platforms - (see http://build.webkit.sed.hu/builders/Qt%20Symbian%20ARMv5%20Release/builds/1156/steps/API%20tests/logs/stdio)
> 
> FAIL!  : tst_QWebView::renderingAfterMaxAndBack() Compared values are not the same
>    Loc: [D:/w/qt-symbian-release/build/Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp(504)]
> 
> Perhaps the test should render smaller sized content - e.g. 50x50 instead of 1024x768. Disabling the scrollbars would be a good ideas as well (see http://trac.webkit.org/changeset/84332)
> 
> CCd Yi who might be able to help with testing on Symbian.

Still failing at build #1774
http://build.webkit.sed.hu/builders/Qt%20Symbian%20ARMv5%20Release/builds/1774/steps/API%20tests/logs/stdio

But is this really a P1 error?

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.