The current main frame/frame view of WebKitWebView is currently never sending a onscroll event. This is due the ScrollView::valueChanged code being mostly dead code...
Created attachment 30133 [details] Make scrolling go through Scrollbar to get the ScrollbarClient invoked proposed approach for the problem, only halfway through though. Please comment anyway.
> Using the wrapped GtkAdjustment approach we might be able to just have > special case for the ScrollView::createScrollbar to create these > scrollbars... [...] > PassRefPtr<ScrollbarGtk> ScrollbarGtk::createScrollbar(ScrollbarClient* client, ScrollbarOrientation orientation, GtkAdjustment* adj) > { > return adoptRef(new ScrollbarGtk(client, orientation, adj)); > } And using this we can ditch the createNativeScrollbar, you mean? > - /* > - * disable the scrollbars (if we have any) as the GtkAdjustment over > - */ I like the patch in general, it's a good cleanup, but this comment has really blown off my mind while I was reading through this code last week, and I believe you could perhaps make it clearer at this point why we need to disable the scrollbars here: > + } else { > + m_horizontalAdjustment = ScrollbarGtk::createScrollbar(this, HorizontalScrollbar, hadj); > + m_verticalAdjustment = ScrollbarGtk::createScrollbar(this, VerticalScrollbar, vadj); > setHasVerticalScrollbar(false); > setHasHorizontalScrollbar(false); [...] > + m_horizontalAdjustment->setSteps(visibleWidth() / 10.0, visibleWidth() * 0.9, 1); > + m_horizontalAdjustment->setProportion(visibleWidth(), contentsWidth()); > + m_horizontalAdjustment->setCurrentPos(scroll.width()); Xan has done some work recently on the scrolling code to use the WebCore constants instead of magic numbers, which I think we should use here as well; See r43390. > + m_verticalAdjustment->setSteps(visibleHeight() / 10.0, visibleHeight() * 0.9, 1); > + m_verticalAdjustment->setProportion(visibleHeight(), contentsHeight()); > + m_verticalAdjustment->setCurrentPos(scroll.height()); Same here. > +// Wrap a GtkAdjustment into a WebCore::Scrollbar to properly use the ScrollbarClient > +// for things like valueChanged. > +ScrollbarGtk::ScrollbarGtk(ScrollbarClient* client, ScrollbarOrientation orientation, GtkAdjustment* adjustment) > + : Scrollbar(client, orientation, RegularScrollbar) > + , m_adjustment(adjustment) > +{ > + // reset the adjustment to a known state > + m_adjustment->lower = 0; > + if (m_adjustment->value != 0) { > + m_adjustment->value = 0; > + gtk_adjustment_value_changed(m_adjustment); > + } Is this wanted? I'm thinking of a javascript call that forces a scrollbar to appear, by using the overflow CSS property, for instance. Will it make the scrolled content go to the top? > + // We have nothing to show as we are solely operating on the GtkAdjustment > + resize(0, 0); I don't get it =(.
(In reply to comment #2) > And using this we can ditch the createNativeScrollbar, you mean? > > > - /* > > - * disable the scrollbars (if we have any) as the GtkAdjustment over > > - */ > > I like the patch in general, it's a good cleanup, but this comment has really > blown off my mind while I was reading through this code last week, and I > believe you could perhaps make it clearer at this point why we need to disable > the scrollbars here: That's because when you set the adjustments you are telling the View that you'll handle the scrolling yourself (for example, through a GtkScrolledWindow), so you want to disable WebKit's scrollbars. I'm not sure why Holger is removing the comment though... > > + m_horizontalAdjustment->setSteps(visibleWidth() / 10.0, visibleWidth() * 0.9, 1); > > + m_horizontalAdjustment->setProportion(visibleWidth(), contentsWidth()); > > + m_horizontalAdjustment->setCurrentPos(scroll.width()); > > Xan has done some work recently on the scrolling code to use the WebCore > constants instead of magic numbers, which I think we should use here as well; > See r43390. /me nods (although the WebCore constants are more 'magic numbers' than what we were using :D) I'll try to look at this tomorrow!
Created attachment 30172 [details] Completely kill the "adjustment" The only special case we have in GTK+ after this patch is that we have a custom ::createScrollbar implementation that will create a ScrollbarGtk to operate on the external GtkAdjustment. This way we will go through the normal Scrollbar paths and fix the sendScrollEvent bug.
Comment on attachment 30172 [details] Completely kill the "adjustment" r=me
Landed in r44177.