WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
131831
[EFL][WK2] Changing page zoom factor does not work correctly if fixed layout is used.
https://bugs.webkit.org/show_bug.cgi?id=131831
Summary
[EFL][WK2] Changing page zoom factor does not work correctly if fixed layout ...
Eunmi Lee
Reported
2014-04-17 17:31:10 PDT
We can test changing page zoom factor with fixed layout using MiniBrowser as follow option.
> MiniBrowser -L 1
But it does not work correctly - page is not zoomed correctly. It's caused by fitting logic of PageViewportController. solution: 1. set user interaction if page zoom is changed. 2. use content size which does not apply page zoom factor.
Attachments
Patch
(2.77 KB, patch)
2014-04-17 17:41 PDT
,
Eunmi Lee
no flags
Details
Formatted Diff
Diff
Patch
(2.84 KB, patch)
2014-05-01 00:08 PDT
,
Eunmi Lee
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Eunmi Lee
Comment 1
2014-04-17 17:41:06 PDT
Created
attachment 229601
[details]
Patch
Gyuyoung Kim
Comment 2
2014-04-30 00:40:35 PDT
Comment on
attachment 229601
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=229601&action=review
> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:246 > + impl->pageViewportController().setHadUserInteraction(true);
I don't think we have to set "true" whenever ewk_view_page_zoom_set() is called. Shouldn't we call it only one time ?
Eunmi Lee
Comment 3
2014-04-30 03:46:55 PDT
Comment on
attachment 229601
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=229601&action=review
>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:246 >> + impl->pageViewportController().setHadUserInteraction(true); > > I don't think we have to set "true" whenever ewk_view_page_zoom_set() is called. Shouldn't we call it only one time ?
we can check hadUserInteraction value and set true only if it is false. I will update patch.
Gyuyoung Kim
Comment 4
2014-04-30 04:12:23 PDT
Comment on
attachment 229601
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=229601&action=review
> Source/WebKit2/UIProcess/WebPageProxy.cpp:3055 > + // We have to find better solution.
Eunmi, as you know, this looks one of workaround solutions. I'm also investigating how to solve this problem fundamentally. If you have spare time with your nice condition, please take a look fundamental problem.
Gyuyoung Kim
Comment 5
2014-04-30 04:14:46 PDT
Comment on
attachment 229601
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=229601&action=review
>> Source/WebKit2/UIProcess/WebPageProxy.cpp:3055 >> + // We have to find better solution. > > Eunmi, as you know, this looks one of workaround solutions. I'm also investigating how to solve this problem fundamentally. If you have spare time with your nice condition, please take a look fundamental problem.
One more comment: I also think that page zoom on fixed layout should work same as "fixed layout is off".
Gyuyoung Kim
Comment 6
2014-04-30 04:25:57 PDT
Comment on
attachment 229601
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=229601&action=review
>>> Source/WebKit2/UIProcess/WebPageProxy.cpp:3055 >>> + // We have to find better solution. >> >> Eunmi, as you know, this looks one of workaround solutions. I'm also investigating how to solve this problem fundamentally. If you have spare time with your nice condition, please take a look fundamental problem. > > One more comment: I also think that page zoom on fixed layout should work same as "fixed layout is off".
Second comment: RenderView::viewWidth() and RenderView::viewHeight() value have been affected by on/off of fixedlayout. When fixed layout is on, with/height look be multiplied by pageZoomFactor. If we don't apply effectiveZoom(), when the fixed layout is enabled without this patch, page zoom works as it is off. int RenderView::viewWidth() const { int width = 0; if (!shouldUsePrintingLayout()) { width = frameView().layoutWidth(); width = frameView().useFixedLayout() ? ceilf(style().effectiveZoom() * float(width)) : width; } ... } RenderView::viewWidth()
http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderView.cpp#L1109
Eunmi Lee
Comment 7
2014-05-01 00:08:53 PDT
Created
attachment 230568
[details]
Patch
Gyuyoung Kim
Comment 8
2014-05-09 03:36:06 PDT
Comment on
attachment 230568
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=230568&action=review
> Source/WebKit2/UIProcess/WebPageProxy.cpp:3119 > + newSize.scale(1 / m_pageZoomFactor);
As mentioned in
comment #6
, RenderView::viewWidth/Height() multiplies the pageZoomFactor(= effectiveZoom()) with width/height when fixed layout is enabled. So, I think we need to check if the calculation logic is correct from EFL port perspective. I think this patch looks a workaround patch. I'd like to clear r?,cq? until finishing the investigation.
Michael Catanzaro
Comment 9
2017-03-11 10:32:31 PST
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug