Bug 105632 – [EFL][WK2] Never create WebCore scrollbars for EFL/WK2

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[Kenneth Rohde Christiansen]

SSIA

[Kenneth Rohde Christiansen]

Created attachment 180507 [details]
Patch

[Antonio Gomes]

Comment on attachment 180507 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1238
> +    m_frame->coreFrame()->createView(webPage->size(), backgroundColor, /* transparent */ false, IntSize(), currentFixedVisibleContentRect, shouldUseFixedLayout, ScrollbarAlwaysOff, true, ScrollbarAlwaysOff, true);

would be nice to have true /*comment*/ here

> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1246
> +    m_frame->coreFrame()->view()->setTransparent(!webPage->drawsBackground());

can not you pass this as one of the parameter of createView?

[Kenneth Rohde Christiansen]

Created attachment 180515 [details]
Patch

[WebKit Review Bot]

Comment on attachment 180515 [details]
Patch

Clearing flags on attachment: 180515

Committed r138378: <http://trac.webkit.org/changeset/138378>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #4)
> (From update of attachment 180515 [details])
> Clearing flags on attachment: 180515
> 
> Committed r138378: <http://trac.webkit.org/changeset/138378>

It made zillion tests fail on EFL-WK2 and Qt-WK2:
 - http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/2026
 - http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Amazon%20EC2%29/builds/12024

Could you check it, please?

[Kenneth Rohde Christiansen]

(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 180515 [details] [details])
> > Clearing flags on attachment: 180515
> > 
> > Committed r138378: <http://trac.webkit.org/changeset/138378>
> 
> It made zillion tests fail on EFL-WK2 and Qt-WK2:
>  - http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/2026
>  - http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Amazon%20EC2%29/builds/12024
> 
> Could you check it, please?

Sure, I will land potential fix.

[Kenneth Rohde Christiansen]

Landed potential fix in 138395

[Csaba Osztrogonác]

(In reply to comment #8)
> Landed potential fix in 138395
Thanks, it fixed the bug I mentioned.

[Allan Sandfeld Jensen]

This patch broke keyboard scrolling. The scrollbars are required for keyboard scrolling since information about linestep and pagestep is calculated in them. Therefore we should let delegated scrolling have scrollbars but just choose not to render them.

[Allan Sandfeld Jensen]

Reopening since this needs to be solved differently. What was the problem with having ScrollBar objects in the first place?

[Antonio Gomes]

(In reply to comment #10)
> This patch broke keyboard scrolling. The scrollbars are required for keyboard scrolling since information about linestep and pagestep is calculated in them. Therefore we should let delegated scrolling have scrollbars but just choose not to render them.

Patch was inspired on BlackBerry port, when scrollbars are not created either.

Keyboard scrolling used to fine there, as far as I can tell. It is not a path very explored, though..

[Antonio Gomes]

(In reply to comment #12)
> (In reply to comment #10)
> > This patch broke keyboard scrolling. The scrollbars are required for keyboard scrolling since information about linestep and pagestep is calculated in them. Therefore we should let delegated scrolling have scrollbars but just choose not to render them.
> 
> Patch was inspired on BlackBerry port, when scrollbars are not created either.
> 
> Keyboard scrolling used to fine there, as far as I can tell. It is not a path very explored, though..

It's been 8 month since patch landed. Are there tests covering keyboard scrolling that EFL does not run?

[Allan Sandfeld Jensen]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > This patch broke keyboard scrolling. The scrollbars are required for keyboard scrolling since information about linestep and pagestep is calculated in them. Therefore we should let delegated scrolling have scrollbars but just choose not to render them.
> > 
> > Patch was inspired on BlackBerry port, when scrollbars are not created either.
> > 
> > Keyboard scrolling used to fine there, as far as I can tell. It is not a path very explored, though..
> 
> It's been 8 month since patch landed. Are there tests covering keyboard scrolling that EFL does not run?

Back when I implemented it I solved wheel-events and keyboard scrolling together. I think only wheel events were tested.

Btw, you can see the problem is ScrollableArea::scroll(), where it exists if no scrollbar is found.

This only happens with delegated scrolling btw. For Qt WK2 we can run in either delegated or non-delated (touch or desktop mode). The desktop mode still works.

[Antonio Gomes]

> Btw, you can see the problem is ScrollableArea::scroll(), where it exists if no scrollbar is found.
> 
> This only happens with delegated scrolling btw. For Qt WK2 we can run in either delegated or non-delated (touch or desktop mode). The desktop mode still works.

That might be a deeper problem to solve: back at the time, I made a set of change decoupling scrolling from scrollbars. Maybe it has regressed.

I remember kenneth testing both wheels and keyboard, but 'll let him to speak up.

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.