SSIA
Created attachment 180507 [details] Patch
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?
Created attachment 180515 [details] Patch
Comment on attachment 180515 [details] Patch Clearing flags on attachment: 180515 Committed r138378: <http://trac.webkit.org/changeset/138378>
All reviewed patches have been landed. Closing bug.
(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?
(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.
Landed potential fix in 138395
(In reply to comment #8) > Landed potential fix in 138395 Thanks, it fixed the bug I mentioned.
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.
Reopening since this needs to be solved differently. What was the problem with having ScrollBar objects in the first place?
(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..
(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?
(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.
> 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.
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.