You can watch this problem after clicking the button of maximize window in MiniBrowser. It looks like page has wrong scale factor. I added logs in PageViewportController::updateMinimumScaleToFit() to fix this issue. Logs are : (when click the maximize button) [PageViewportController::updateMinimumScaleToFit] m_viewportSize width:1920.000000 height:964.000000 [PageViewportController::updateMinimumScaleToFit] m_contentsSize width:800.000000 height:560.000000 [PageViewportController::updateMinimumScaleToFit] currentlyScaledToFit:1 [PageViewportController::updateMinimumScaleToFit] minimumScale:2.400000 [PageViewportController::updateMinimumScaleToFit] m_minimumScaleToFit:1.000000 [PageViewportController::applyScaleAfterRenderingContents] scale:2.400000 minimumScale updated the wrong value because the size of viewport was not the same with contents.
Created attachment 243791 [details] Patch
I try to update the size of contents immediately to fix this problem but it still occurred at www.naver.com. I will try to find the cause to fix this issue.
Created attachment 243818 [details] Patch
After attachment 243818 [details], Page was scaled correctly when clicked the maxmize button in MiniBrowser. Gyuyoung, Could I ask why you removed m_hadUserInteraction in updateMinimumScaleToFit()? But api test still failed in EWK2ViewTest.ewk_view_scale_with_fixed_layout. ../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:917: Failure Value of: ewk_view_scale_get(webView()) Actual: 0.81632656 Expected: 1 916: // Default scale value is 1.0. 917: ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView()));
(In reply to comment #4) > After attachment 243818 [details], Page was scaled correctly when clicked > the maxmize button in MiniBrowser. > > Gyuyoung, Could I ask why you removed m_hadUserInteraction in > updateMinimumScaleToFit()? IIRC, I removed it because it seems we don't need to use it. If we need to use it, we can revert it. > But api test still failed in EWK2ViewTest.ewk_view_scale_with_fixed_layout. > > ../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:917: Failure > Value of: ewk_view_scale_get(webView()) > Actual: 0.81632656 > Expected: 1 > > 916: // Default scale value is 1.0. > 917: ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); Hmm, this test was passed when I landed r176631. I guess following patch made this regression.
Created attachment 244432 [details] Patch
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 > - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 > - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); This is to test that webview can keep previous scale factor after reloading a page. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-924 > - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.5, 0, 0)); ditto.
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 >> - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); > > If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? > > ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); If load the defaultTestPage, pageScaleFactor was changed from 1.0 to 0.81632656 because default contentSize(980x735) bigger then default viewSize(800x560). So I removed the "loadUrlSync(environment->defaultTestPageUrl())" to check the default value of pageScaleFactor whether it is 1.0. >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 >> - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); > > This is to test that webview can keep previous scale factor after reloading a page. If ewk_view_scale_set() was invoked before loadUrl(), it didn't set pageScaleFactor because syncVisibleContents() failed. If syncVisibleContents() failed, it recover the original pageScaleFactor that was old one. So I moved the ewk_view_scale_set() after invoking the loadUrlSync().
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review >>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 >>> - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); >> >> If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? >> >> ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); > > If load the defaultTestPage, pageScaleFactor was changed from 1.0 to 0.81632656 because default contentSize(980x735) bigger then default viewSize(800x560). > So I removed the "loadUrlSync(environment->defaultTestPageUrl())" to check the default value of pageScaleFactor whether it is 1.0. If so, we have to change the default content size with 800x560. I think you change test behavior without reasonable reason. >>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 >>> - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); >> >> This is to test that webview can keep previous scale factor after reloading a page. > > If ewk_view_scale_set() was invoked before loadUrl(), it didn't set pageScaleFactor because syncVisibleContents() failed. > If syncVisibleContents() failed, it recover the original pageScaleFactor that was old one. > So I moved the ewk_view_scale_set() after invoking the loadUrlSync(). I don't understand well. We already load a default test page in 914 line before you removed it. If so, ewk_view_scale_set(webView(), 1.2, 0, 0) should not be failed, isn't it ?
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review >>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 >>>> - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); >>> >>> If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? >>> >>> ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); >> >> If load the defaultTestPage, pageScaleFactor was changed from 1.0 to 0.81632656 because default contentSize(980x735) bigger then default viewSize(800x560). >> So I removed the "loadUrlSync(environment->defaultTestPageUrl())" to check the default value of pageScaleFactor whether it is 1.0. > > If so, we have to change the default content size with 800x560. I think you change test behavior without reasonable reason. Yes, ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())) passed if I changed the default page width from 980 to 800 in ViewportConfiguration::webpageParameters(). Should I changed the value in EFL port? >>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 >>>> - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); >>> >>> This is to test that webview can keep previous scale factor after reloading a page. >> >> If ewk_view_scale_set() was invoked before loadUrl(), it didn't set pageScaleFactor because syncVisibleContents() failed. >> If syncVisibleContents() failed, it recover the original pageScaleFactor that was old one. >> So I moved the ewk_view_scale_set() after invoking the loadUrlSync(). > > I don't understand well. We already load a default test page in 914 line before you removed it. If so, ewk_view_scale_set(webView(), 1.2, 0, 0) should not be failed, isn't it ? Yes, it is. But it failed if I changed the default page width to 800. ../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:922: Failure Value of: ewk_view_scale_get(webView()) Actual: 0.81632656 Expected: 1.2 I think ewk_view_scale_set(webView(), 1.2, 0, 0) didn'set the pageScaleFactor because syncVisibleContents() failed. So it recover the old pageScaleFactor but I don't know why actual value is 0.81632656.
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review >>>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 >>>>> - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); >>>> >>>> If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? >>>> >>>> ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); >>> >>> If load the defaultTestPage, pageScaleFactor was changed from 1.0 to 0.81632656 because default contentSize(980x735) bigger then default viewSize(800x560). >>> So I removed the "loadUrlSync(environment->defaultTestPageUrl())" to check the default value of pageScaleFactor whether it is 1.0. >> >> If so, we have to change the default content size with 800x560. I think you change test behavior without reasonable reason. > > Yes, ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())) passed if I changed the default page width from 980 to 800 in ViewportConfiguration::webpageParameters(). > Should I changed the value in EFL port? When I landed r176631, this API test was passed. Could you check if content Size was also 980x735 at that time ? >>>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 >>>>> - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); >>>> >>>> This is to test that webview can keep previous scale factor after reloading a page. >>> >>> If ewk_view_scale_set() was invoked before loadUrl(), it didn't set pageScaleFactor because syncVisibleContents() failed. >>> If syncVisibleContents() failed, it recover the original pageScaleFactor that was old one. >>> So I moved the ewk_view_scale_set() after invoking the loadUrlSync(). >> >> I don't understand well. We already load a default test page in 914 line before you removed it. If so, ewk_view_scale_set(webView(), 1.2, 0, 0) should not be failed, isn't it ? > > Yes, it is. But it failed if I changed the default page width to 800. > > ../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:922: Failure > Value of: ewk_view_scale_get(webView()) > Actual: 0.81632656 > Expected: 1.2 > > I think ewk_view_scale_set(webView(), 1.2, 0, 0) didn'set the pageScaleFactor because syncVisibleContents() failed. > So it recover the old pageScaleFactor but I don't know why actual value is 0.81632656. I made to restore previous scale factor if sysnVisibleContents() is failed. I think we should not keep the failing pageScaleFactor in PageViewportController though coordinatedGraphics fails to change scale. I think we should check why syncVisibleContents() is failed first.
Comment on attachment 244432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244432&action=review >>>>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-914 >>>>>> - ASSERT_TRUE(loadUrlSync(environment->defaultTestPageUrl())); >>>>> >>>>> If we don't load a test page, what is meaning to check if default scale value is 1.0 below ? >>>>> >>>>> ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())); >>>> >>>> If load the defaultTestPage, pageScaleFactor was changed from 1.0 to 0.81632656 because default contentSize(980x735) bigger then default viewSize(800x560). >>>> So I removed the "loadUrlSync(environment->defaultTestPageUrl())" to check the default value of pageScaleFactor whether it is 1.0. >>> >>> If so, we have to change the default content size with 800x560. I think you change test behavior without reasonable reason. >> >> Yes, ASSERT_FLOAT_EQ(1, ewk_view_scale_get(webView())) passed if I changed the default page width from 980 to 800 in ViewportConfiguration::webpageParameters(). >> Should I changed the value in EFL port? > > When I landed r176631, this API test was passed. Could you check if content Size was also 980x735 at that time ? I tried to test the ewk_view_scale_with_fixed_layout after reset r176631. Failed in 5 times among the 10 times. Maybe,,there are some timing issue. Value of: ewk_view_scale_get(webView()) Actual: 0.81632656 Expected: 1.5 Value of: ewk_view_scale_get(webView()) Actual: 0.81632656 Expected: 1.2 Result is different whenever I try to test. And I checked the content size in PageViewportController::updateMinimumScaleToFit(). contentSize width:980.000000 height:735.000000 m_viewportSize width:800.000000 height:600.000000 minimumScale:0.816327 >>>>>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-919 >>>>>> - ASSERT_TRUE(ewk_view_scale_set(webView(), 1.2, 0, 0)); >>>>> >>>>> This is to test that webview can keep previous scale factor after reloading a page. >>>> >>>> If ewk_view_scale_set() was invoked before loadUrl(), it didn't set pageScaleFactor because syncVisibleContents() failed. >>>> If syncVisibleContents() failed, it recover the original pageScaleFactor that was old one. >>>> So I moved the ewk_view_scale_set() after invoking the loadUrlSync(). >>> >>> I don't understand well. We already load a default test page in 914 line before you removed it. If so, ewk_view_scale_set(webView(), 1.2, 0, 0) should not be failed, isn't it ? >> >> Yes, it is. But it failed if I changed the default page width to 800. >> >> ../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:922: Failure >> Value of: ewk_view_scale_get(webView()) >> Actual: 0.81632656 >> Expected: 1.2 >> >> I think ewk_view_scale_set(webView(), 1.2, 0, 0) didn'set the pageScaleFactor because syncVisibleContents() failed. >> So it recover the old pageScaleFactor but I don't know why actual value is 0.81632656. > > I made to restore previous scale factor if sysnVisibleContents() is failed. I think we should not keep the failing pageScaleFactor in PageViewportController though coordinatedGraphics fails to change scale. I think we should check why syncVisibleContents() is failed first. I checked the syncVisiblieContents() whether is failed or not. It is not a problem of syncVisibleContent(),,, syncVisibleContents() isn't failed in test. I will find what is problem.
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.