99751 2012-10-18 13:28:01 -0700 PageViewportController::m_rawAttributes.devicePixelRatio is not initialized 2012-10-19 05:27:51 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 yael webkit-unassigned abecsi kenneth tonikitoo webkit.review.bot oldest_to_newest 745507 0 yael 2012-10-18 13:28:01 -0700 PageViewportController::m_rawAttributes.devicePixelRatio is not initialized and can cause the zoom level to become infinity. It seems in qt port it starts with garbage value but in EFL port starts with 0, and then the zoom level is set to infinity in computeMinimumScaleFactorForContentContained . Once minimum scale starts as infinity, it never goes down. 745512 1 169458 yael 2012-10-18 13:37:58 -0700 Created attachment 169458 Patch 745569 2 169458 webkit.review.bot 2012-10-18 14:20:20 -0700 Comment on attachment 169458 Patch Clearing flags on attachment: 169458 Committed r131802: <http://trac.webkit.org/changeset/131802> 745570 3 webkit.review.bot 2012-10-18 14:20:23 -0700 All reviewed patches have been landed. Closing bug. 746125 4 169458 kenneth 2012-10-19 01:34:08 -0700 Comment on attachment 169458 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review > Source/WebKit2/UIProcess/PageViewportController.cpp:74 > + m_rawAttributes.devicePixelRatio = 1; ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else 746145 5 abecsi 2012-10-19 02:08:07 -0700 (In reply to comment #4) > (From update of attachment 169458 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review > > > Source/WebKit2/UIProcess/PageViewportController.cpp:74 > > + m_rawAttributes.devicePixelRatio = 1; > > ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect. I'll prepare a patch to remove it from the attributes. 746231 6 yael 2012-10-19 05:17:25 -0700 (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 169458 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review > > > > > Source/WebKit2/UIProcess/PageViewportController.cpp:74 > > > + m_rawAttributes.devicePixelRatio = 1; > > > > ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else > > Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect. > > I'll prepare a patch to remove it from the attributes. Well, without this patch, I saw a black screen in my version of EFL MIniBrowser, after hooking it up with PageViewController. The reason was that the zoom level was infinity. m_rawAttributes.devicePixelRatio is actually used in computeMinimumScaleFactorForContentContained (ViewportAgruments.cpp) 746233 7 abecsi 2012-10-19 05:27:51 -0700 (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 169458 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review > > > > > > > Source/WebKit2/UIProcess/PageViewportController.cpp:74 > > > > + m_rawAttributes.devicePixelRatio = 1; > > > > > > ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else > > > > Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect. > > > > I'll prepare a patch to remove it from the attributes. > > Well, without this patch, I saw a black screen in my version of EFL MIniBrowser, after hooking it up with PageViewController. > The reason was that the zoom level was infinity. > > m_rawAttributes.devicePixelRatio is actually used in computeMinimumScaleFactorForContentContained (ViewportAgruments.cpp) Yes, you are right restrictMinimumScaleFactorToViewportSize and computeMinimumScaleFactorForContentContained could be called before didChangeViewportAttributes. 169458 2012-10-18 13:37:58 -0700 2012-10-19 01:34:08 -0700 Patch 99751.patch text/plain 1759 yael SW5kZXg6IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9QYWdlVmlld3BvcnRDb250cm9sbGVyLmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvUGFnZVZpZXdwb3J0Q29u dHJvbGxlci5jcHAJKHJldmlzaW9uIDEzMDY1MCkKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9QYWdlVmlld3BvcnRDb250cm9sbGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNjksOCArNjks OSBAQAogICAgICwgbV9oYWRVc2VySW50ZXJhY3Rpb24oZmFsc2UpCiAgICAgLCBtX2VmZmVjdGl2 ZVNjYWxlKDEpCiB7Ci0gICAgLy8gSW5pdGlhbGl6aW5nIFZpZXdwb3J0IFJhdyBBdHRyaWJ1dGVz IHRvIGF2b2lkIHJhbmRvbSBuZWdhdGl2ZSBzY2FsZSBmYWN0b3JzCisgICAgLy8gSW5pdGlhbGl6 aW5nIFZpZXdwb3J0IFJhdyBBdHRyaWJ1dGVzIHRvIGF2b2lkIHJhbmRvbSBuZWdhdGl2ZSBvciBp bmZpbml0eSBzY2FsZSBmYWN0b3JzCiAgICAgLy8gaWYgdGhlcmUgaXMgYSByYWNlIGNvbmRpdGlv biBiZXR3ZWVuIHRoZSBmaXJzdCBsYXlvdXQgYW5kIHNldHRpbmcgdGhlIHZpZXdwb3J0IGF0dHJp YnV0ZXMgZm9yIHRoZSBmaXJzdCB0aW1lLgorICAgIG1fcmF3QXR0cmlidXRlcy5kZXZpY2VQaXhl bFJhdGlvID0gMTsKICAgICBtX3Jhd0F0dHJpYnV0ZXMuaW5pdGlhbFNjYWxlID0gMTsKICAgICBt X3Jhd0F0dHJpYnV0ZXMubWluaW11bVNjYWxlID0gMTsKICAgICBtX3Jhd0F0dHJpYnV0ZXMubWF4 aW11bVNjYWxlID0gMTsKSW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzMTc5NykKKysrIFNvdXJj ZS9XZWJLaXQyL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEy LTEwLTE4ICBZYWVsIEFoYXJvbiAgPHlhZWwuYWhhcm9uQGludGVsLmNvbT4KKworICAgICAgICBQ YWdlVmlld3BvcnRDb250cm9sbGVyOjptX3Jhd0F0dHJpYnV0ZXMuZGV2aWNlUGl4ZWxSYXRpbyBp cyBub3QgaW5pdGlhbGl6ZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTk5NzUxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgUGFnZVZpZXdwb3J0Q29udHJvbGxlcjo6bV9yYXdBdHRyaWJ1dGVzLmRldmljZVBp eGVsUmF0aW8gbmVlZHMgdG8gYmUgaW5pdGlhbGl6ZWQuCisgICAgICAgIER1ZSB0byByYWNlIGNv bmRpdGlvbiwgd2UgY291bGQgc2V0IHRoZSB2aWV3cG9ydCBhdHRyaWJ1dGVzIGJlZm9yZSB0aGUg Zmlyc3QgbGF5b3V0LgorICAgICAgICBJZiB0aGF0IGhhcHBlbnMsIFBhZ2VWaWV3cG9ydENvbnRy b2xsZXI6Om1fcmF3QXR0cmlidXRlcy5kZXZpY2VQaXhlbFJhdGlvCisgICAgICAgIGlzIHVzZWQg d2l0aG91dCBiZWluZyBpbml0aWFsaXplZC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9QYWdlVmll d3BvcnRDb250cm9sbGVyLmNwcDoKKyAgICAgICAgKFdlYktpdDo6UGFnZVZpZXdwb3J0Q29udHJv bGxlcjo6UGFnZVZpZXdwb3J0Q29udHJvbGxlcik6CisKIDIwMTItMTAtMTggIEFuZGVycyBDYXJs c3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBCdWlsZCBmaXguCg==