Bug 86819

Summary: [chromium] Support mobile device rotation resizing
Product: WebKit Reporter: Alexandre Elias <aelias>
Component: New BugsAssignee: Alexandre Elias <aelias>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aelias, apavlov, dglazkov, fsamuel, jamesr, johnme, vsevik, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 87468    
Bug Blocks: 66687    
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ec2-cr-linux-03
none
Patch
none
Patch none

Alexandre Elias
Reported 2012-05-18 00:18:32 PDT
[chromium] Support mobile device rotation resizing
Attachments
Patch (4.11 KB, patch)
2012-05-18 00:29 PDT, Alexandre Elias 		no flags
DetailsFormatted DiffDiff
Patch (4.01 KB, patch)
2012-05-23 14:47 PDT, Alexandre Elias 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ec2-cr-linux-03 (712.21 KB, application/zip)
2012-05-23 16:03 PDT, WebKit Review Bot 		no flags
Details
Patch (4.00 KB, patch)
2012-05-23 16:55 PDT, Alexandre Elias 		no flags
DetailsFormatted DiffDiff
Patch (4.05 KB, patch)
2012-06-19 19:04 PDT, Alexandre Elias 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Alexandre Elias
Comment 1 2012-05-18 00:29:26 PDT
Created attachment 142653 [details] Patch
Adam Barth
Comment 2 2012-05-18 04:49:02 PDT
Comment on attachment 142653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142653&action=review This looks reasonable, but I'm not an expert on such things. Is there someone else you'd like to review your patch as well? > Source/WebKit/chromium/src/WebViewImpl.cpp:1350 > + int dpiIndependentViewportWidth = newSize.width / > + page()->settings()->defaultDeviceScaleFactor(); > + settings()->setLayoutFallbackWidth( > + std::max(kStandardFallbackWidth, dpiIndependentViewportWidth)); There's no 80 col limit in WebKit. These can each be on one line.
James Robinson
Comment 3 2012-05-21 17:37:25 PDT
Comment on attachment 142653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142653&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:1346 > + const int kStandardFallbackWidth = 980; we don't typically use the "k" prefix for constants in WebKit - constants are named the same way as normal variables
Alexandre Elias
Comment 4 2012-05-23 14:47:09 PDT
Created attachment 143656 [details] Patch
Alexandre Elias
Comment 5 2012-05-23 14:47:31 PDT
Syntax problems fixed.
WebKit Review Bot
Comment 6 2012-05-23 16:03:46 PDT
Comment on attachment 143656 [details] Patch Attachment 143656 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12780310 New failing tests: WebFrameTest.DeviceScaleFactorUsesDefaultWithoutViewportTag WebFrameTest.FixedLayoutInitializeAtMinimumPageScale
WebKit Review Bot
Comment 7 2012-05-23 16:03:56 PDT
Created attachment 143667 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Alexandre Elias
Comment 8 2012-05-23 16:55:27 PDT
Created attachment 143681 [details] Patch
Alexandre Elias
Comment 9 2012-05-23 16:56:34 PDT
Ouch, while fixing style warnings to remove "== 0", I had forgotten to replace them with ! symbols. Good thing these unit tests just landed.
WebKit Review Bot
Comment 10 2012-05-24 18:56:26 PDT
Comment on attachment 143681 [details] Patch Clearing flags on attachment: 143681 Committed r118461: <http://trac.webkit.org/changeset/118461>
WebKit Review Bot
Comment 11 2012-05-24 18:56:32 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 12 2012-05-25 00:49:40 PDT
Re-opened since this is blocked by 87468
Vsevolod Vlasov
Comment 14 2012-05-25 01:20:35 PDT
AutomationRendererHelperTest.RTLSnapshot: chrome/renderer/automation/automation_renderer_helper_browsertest.cc:40: Failure Value of: base::MD5String(png_data_str).c_str() Actual: "d81f041da73f0e5eee86ecb995410b4b" Expected: reference_md5.c_str() Which is: "3adefecb4472e6ba1812f9af1fdea3e4" AutomationRendererHelperTest.ScrollingSnapshot: chrome/renderer/automation/automation_renderer_helper_browsertest.cc:40: Failure Value of: base::MD5String(png_data_str).c_str() Actual: "d81f041da73f0e5eee86ecb995410b4b" Expected: reference_md5.c_str() Which is: "3adefecb4472e6ba1812f9af1fdea3e4"
Alexandre Elias
Comment 15 2012-06-19 19:04:03 PDT
Created attachment 148485 [details] Patch
Alexandre Elias
Comment 16 2012-06-19 19:06:06 PDT
OK, replaced the fixedLayoutEnabled() guards with settings()->viewportEnabled() instead, because AutomationRendererHelper is using fixed layout for the purpose of grabbing a snapshot of the contents while having resizes ignored.
WebKit Review Bot
Comment 17 2012-06-20 15:58:15 PDT
Comment on attachment 148485 [details] Patch Clearing flags on attachment: 148485 Committed r120877: <http://trac.webkit.org/changeset/120877>
WebKit Review Bot
Comment 18 2012-06-20 15:58:20 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.