Bug 49177 – [GTK] fast/events/scroll-after-click-on-tab-index has been failing on the bots

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

Here's the diff:

--- /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/events/scroll-after-click-on-tab-index-expected.txt	2010-11-08 07:07:21.665738800 -0800
+++ /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/events/scroll-after-click-on-tab-index-actual.txt	2010-11-08 07:07:21.665738800 -0800
@@ -4,7 +4,7 @@
 
 TEST COMPLETE
 Scroll position is more than 0
-PASS document.body.scrollTop > 0 is true
+FAIL document.body.scrollTop > 0 should be true. Was false.

[Martin Robinson]

The issue here seems to be that dead scrollbars (ones that have been removed from the main frame) can still control the WebCore's scrollbars after they have been removed. In this case dead scrollbars were stomping on the scrollbar value.

[Martin Robinson]

Created attachment 79018 [details]
Patch

[Xan Lopez]

Comment on attachment 79018 [details]
Patch

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

r=me

> Source/WebCore/platform/gtk/ScrollViewGtk.cpp:84
> +    // we must to inform main frame scrollbars that their adjustments no longer control the

s/we must to/we must/

[Martin Robinson]

Committed r75914: <http://trac.webkit.org/changeset/75914>

[Martin Robinson]

Comment on attachment 79018 [details]
Patch

Thanks for the review! Fixed and landed.

[Philippe Normand]

This patch made the 3 GTK bots really unhappy. Gonna roll out, sorry!

[Philippe Normand]

Reverted r75914 for reason:

multiple crashes on GTK

Committed r75924: <http://trac.webkit.org/changeset/75924>

[Martin Robinson]

(In reply to comment #7)
> Reverted r75914 for reason:
> multiple crashes on GTK
> Committed r75924: <http://trac.webkit.org/changeset/75924>

Thanks for the rollout. I'll take another look soon.

[Martin Robinson]

(In reply to comment #8)
> (In reply to comment #7)
> > Reverted r75914 for reason:
> > multiple crashes on GTK
> > Committed r75924: <http://trac.webkit.org/changeset/75924>
> Thanks for the rollout. I'll take another look soon.

It seems there are some cases where non-mainframe scrollbars do not have a parent. I should have a new, even-simpler fix for this issue posted soon.

[Martin Robinson]

Created attachment 79143 [details]
Patch

[Martin Robinson]

I've uploaded a new patch which makes the determination if a scrollbar is defunct lazily. It seems there is no reasonable way to determine when the child is removed whether or not it is a main frame scrollbar.

[Eric Seidel (no email)]

I feel like Jamesr recently fixed a parent-less scrollbars issue involving tables on some brazilian photos-of-women-being-stupid site the other day.  I wonder if that's at all related to the case where you're seeing scrollbars w/o parents.

[James Robinson]

Adding some scrollbar people.  Offhand, I'm not totally sure what's going on here :)

[Peter Kasting]

Sorry, I don't think I can offer any insight :(

[Martin Robinson]

(In reply to comment #13)
> Adding some scrollbar people.  Offhand, I'm not totally sure what's going on here :)

Just to clarify there are two issues here:

1. The issue of the bug itself, which comes about because GTK+ mainframe scrollbars are native and the WebCore state is controlled asynchronously via GtkAdjustments.

2. The reason for failure of my first attempt, which is that at certain points in the mainframe scrollbar's lifetime it doesn't have a parent. I'm not entirely sure if this is a bug or not, but my second patch does not have that assumption any longer.

[Xan Lopez]

Comment on attachment 79143 [details]
Patch

OK, seems reasonable to me.

[Martin Robinson]

Committed r79349: <http://trac.webkit.org/changeset/79349>