Bug 51892 – [Qt] QtTestBrowser renders only part of page when navigating back

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Robert Hogan]

To recreate:

- Search for something in google.com
- Scroll down the page so the top of the results page is no longer visible
- click on a link
- hit browser->back

Result: only the link you clicked on from the previous page is displayed, the rest of the page is still the page you navigated away from

[Antonio Gomes]

Robert, x11 or raster graphic backend?

[Robert Hogan]

(In reply to comment #1)
> Robert, x11 or raster graphic backend?

X11 on an ATI card using an ATI driver. All kubuntu packages.

[Benjamin Poulain]

(In reply to comment #2)
> (In reply to comment #1)
> > Robert, x11 or raster graphic backend?
> 
> X11 on an ATI card using an ATI driver. All kubuntu packages.

Is it with trunk?
Could that be the driver?
I have not seen anything like that on my machine.

[Aparna Nandyal]

This bug is reproducible using QtTestBrowser and also arora browser. It looks like a regression. It is not happening with oooolder release(which is about 4 months old and it part of Qt release) Not sure how I can get the webkit version number.
I have figured out the cause of the problem. A messed up variable value in rendering is causing this problem. I should have solution for it soon.

[Aparna Nandyal]

Created attachment 80809 [details]
Patch of review

Analysis:
The bug is caused due to erroneous repaint region calculation. Earlier (before http://trac.webkit.org/changeset/73385 to fix https://bugs.webkit.org/show_bug.cgi?id=49220), visibleLayoutOverflow was returned in RenderBox::clippedOverflowRectForRepaint which was a union of layoutOverflow and visualOverflow. The layoutOverflow had the greater rect height as it is intended for scrolling.

If visualOverflow is returned in RenderBox::clippedOverflowRect  the height of the rect is from 0 to unscrolled visible area height.

When user decides to navigate back to a scrolled page in
FrameView::repaintContentRectangle (const IntRect& r, bool immediate)
{
.........

// The value of r is coming from RenderBox::clippedOverflowRect
InrtRect paintRect = r;

// The paintRect is becoming 0 as there is no common area,
// the height of paintRect represents unscrolled height and height in
// visibleContentRect() represents scrolled area height, hence
// painting is not happening
paintRect.intersect(visibleContentRect());

if (paintRect.isEmpty())
return;
...
}


Additional Info:
Bug also reproducible in arora

[Antonio Gomes]

Can not it be tested? It affects everybody.

Hyatt?

[Benjamin Poulain]

(In reply to comment #6)
> Can not it be tested? It affects everybody.

I think we could have autotest for Qt. And I guess pixel tests for the other platforms.

[Aparna Nandyal]

Created attachment 81999 [details]
Tests for the bug

Though this bug is reproducible but when we try to take a screenshot, the png when saved and opened appears to be correct. Only when seen visually the difference can be spotted. Attaching the layout test. To run the test:
1. untar the content
2. From QtTestBrowser launch paint.html (Page 1)
3. It will navigate into another page (Page 2) and come back.
4. Once its back to the original page, you can notice that the content of the Page 1 and Page 2 is overlapping
5. Using QtTestBrowser take a screenshot and check the screenshot, we will not be able to observe the problem.

Screen shot taken through run-webkit-tests --pixel also has the same problem. I can probably raise a bug on the tool but wondering if there is any other way to do a layout test.

[Eric Seidel (no email)]

Comment on attachment 80809 [details]
Patch of review

How do we test this?

[Alexis Menard (darktears)]

Created attachment 86169 [details]
Auto test for Qt.

I have attached an auto-test for Qt. It reproduces the issue but even with the patch applied I can still reproduce the problem inside QtTestBrowser and the test case still fail :(.

[Alexis Menard (darktears)]

Comment on attachment 86169 [details]
Auto test for Qt.

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

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:496
> +    image3.save("test.png");

Obviously this should be removed :)

[Aparna Nandyal]

Created attachment 86194 [details]
Auto test sample for the bug

This Qt test represents the flow that causes the bug.
I have added wait's in between to be able to see the bug visually. 

Without the patch, the bug can be seen and when patch is applied, the bug is no longer seen with this test.

[Aparna Nandyal]

Created attachment 86195 [details]
Html file for the above Qt test

This html file needs to be added into tests/qwebframe/resources/

[Aparna Nandyal]

> I have attached an auto-test for Qt. It reproduces the issue but even with the patch applied I can still reproduce the problem inside QtTestBrowser and the test case still fail :(.

Your auto test actually tests a different bug scenario. Raised https://bugs.webkit.org/show_bug.cgi?id=56669 to track the issue in your auto test.

[Alexis Menard (darktears)]

with a recent trunk I can't reproduce anymore and the test seems to not show the problem.

[Aparna Nandyal]

Confirmed that bug is not reproducible on trunk.

[Csaba Osztrogonác]

I can confirm that it works on trunk now, but unfortunately we got 
a regression with the fix, because the vertical position of the 
previous page isn't restored.

I'm going to find which revision fixed and broke 
them. It might be two different chageset. :)

[Csaba Osztrogonác]

(In reply to comment #17)
> I can confirm that it works on trunk now, but unfortunately we got 
> a regression with the fix, because the vertical position of the 
> previous page isn't restored.
> 
> I'm going to find which revision fixed and broke 
> them. It might be two different chageset. :)

I got it: http://trac.webkit.org/changeset/82185

[Csaba Osztrogonác]

I cc-ed Beth and Darin. 

r82185 fixed an annoying bug (missing rendering), but 
broke a good feature (vertical postion of previous page)

Could you check it, please?

[Beth Dakin]

(In reply to comment #19)
> I cc-ed Beth and Darin. 
> 
> r82185 fixed an annoying bug (missing rendering), but 
> broke a good feature (vertical postion of previous page)
> 
> Could you check it, please?

Sorry, I saw that was broken and haven't filed a bug for it yet. I will file one now.

[Beth Dakin]

(In reply to comment #20)
> (In reply to comment #19)
> > I cc-ed Beth and Darin. 
> > 
> > r82185 fixed an annoying bug (missing rendering), but 
> > broke a good feature (vertical postion of previous page)
> > 
> > Could you check it, please?
> 
> Sorry, I saw that was broken and haven't filed a bug for it yet. I will file one now.

Filed: https://bugs.webkit.org/show_bug.cgi?id=57898

[Csaba Osztrogonác]

(In reply to comment #21)
> Sorry, I saw that was broken and haven't filed a bug for it yet. I will file one now.
> 
> Filed: https://bugs.webkit.org/show_bug.cgi?id=57898

Not a problem, thanks for fixing these bugs. So we 
can close this one, and let's see the new one. ;)