Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
7619015:
Enabled scaling zoom for TOUCH_UI. (Closed)
|
Created:
9 years, 4 months ago by Fady Samuel Modified:
9 years, 4 months ago CC:
chromium-reviews, joi+watch-content_chromium.org, rjkroege Visibility:
Public. |
DescriptionEnabled scaling zoom for TOUCH_UI.
This patch depends on landing this WebKit patch first:
https://bugs.webkit.org/show_bug.cgi?id=66067
BUG=none
TEST=manually
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96694
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97052
Patch Set 1 #
Total comments: 3
Patch Set 2 : Changes as suggested by jam #Messages
Total messages: 17 (0 generated)
Hi John, Could you please take a look at this? I'm waiting to land the associated webkit patch before this can land. This is a temporary hack to enable TOUCH_UI engineers to test and report scaling bugs more easily (by replacing existing zoom functionality). Thanks, Fady
I'm not sure I understand the problem. Anytime I see "temporary hack" in a patch, I wonder why this couldn't be just kept on developer machines and not committed?
On 2011/08/11 18:55:45, John Abd-El-Malek wrote: > I'm not sure I understand the problem. Anytime I see "temporary hack" in a > patch, I wonder why this couldn't be just kept on developer machines and not > committed? I'd like TOUCH_UI developers to get an opportunity to fiddle with scaling zoom easily and report bugs rather than keeping this local.
On 2011/08/11 18:59:49, fsamuel wrote: > On 2011/08/11 18:55:45, John Abd-El-Malek wrote: > > I'm not sure I understand the problem. Anytime I see "temporary hack" in a > > patch, I wonder why this couldn't be just kept on developer machines and not > > committed? > > I'd like TOUCH_UI developers to get an opportunity to fiddle with scaling zoom > easily and report bugs rather than keeping this local. I'm not familiar with the problem, so that's why I was hoping for some background. i.e. how is the scaling changed normally?
On 2011/08/11 18:59:49, fsamuel wrote: > On 2011/08/11 18:55:45, John Abd-El-Malek wrote: > > I'm not sure I understand the problem. Anytime I see "temporary hack" in a > > patch, I wonder why this couldn't be just kept on developer machines and not > > committed? > > I'd like TOUCH_UI developers to get an opportunity to fiddle with scaling zoom > easily and report bugs rather than keeping this local. Scaling zoom magnifies and shrinks the page without affecting flow or layout. Standard zoom reflows the content of the page.
On 2011/08/11 19:07:49, fsamuel wrote: > On 2011/08/11 18:59:49, fsamuel wrote: > > On 2011/08/11 18:55:45, John Abd-El-Malek wrote: > > > I'm not sure I understand the problem. Anytime I see "temporary hack" in a > > > patch, I wonder why this couldn't be just kept on developer machines and not > > > committed? > > > > I'd like TOUCH_UI developers to get an opportunity to fiddle with scaling zoom > > easily and report bugs rather than keeping this local. > > Scaling zoom magnifies and shrinks the page without affecting flow or layout. > Standard zoom reflows the content of the page. I'm just waiting for fishd to review my patch on the webkit side now.
I still don't understand how scaling is changed normally? On Thu, Aug 11, 2011 at 1:17 PM, <fsamuel@chromium.org> wrote: > On 2011/08/11 19:07:49, fsamuel wrote: > >> On 2011/08/11 18:59:49, fsamuel wrote: >> > On 2011/08/11 18:55:45, John Abd-El-Malek wrote: >> > > I'm not sure I understand the problem. Anytime I see "temporary hack" >> in a >> > > patch, I wonder why this couldn't be just kept on developer machines >> and >> > not > >> > > committed? >> > >> > I'd like TOUCH_UI developers to get an opportunity to fiddle with >> scaling >> > zoom > >> > easily and report bugs rather than keeping this local. >> > > Scaling zoom magnifies and shrinks the page without affecting flow or >> layout. >> Standard zoom reflows the content of the page. >> > > I'm just waiting for fishd to review my patch on the webkit side now. > > > http://codereview.chromium.**org/7619015/<http://codereview.chromium.org/7619... >
(Fady and I chatted, and now I better understand why this is needed) http://codereview.chromium.org/7619015/diff/1/content/renderer/render_view.cc File content/renderer/render_view.cc (right): http://codereview.chromium.org/7619015/diff/1/content/renderer/render_view.cc... content/renderer/render_view.cc:3331: #ifndef TOUCH_UI nit: normal convention is "!defined(TOUCH_UI)" http://codereview.chromium.org/7619015/diff/1/content/renderer/render_view.cc... content/renderer/render_view.cc:3359: page_scale_factor = old_page_scale_factor + (function > 0 ? 0.1 : -0.1); these all need to be constants
http://codereview.chromium.org/7619015/diff/1/content/renderer/render_view.cc File content/renderer/render_view.cc (right): http://codereview.chromium.org/7619015/diff/1/content/renderer/render_view.cc... content/renderer/render_view.cc:3331: #ifndef TOUCH_UI On 2011/08/11 20:44:56, John Abd-El-Malek wrote: > nit: normal convention is "!defined(TOUCH_UI)" Done.
lgtm
Change committed as 96694
On 2011/08/13 06:55:32, I haz the power (commit-bot) wrote: > Change committed as 96694 This was reverted because, for whatever reason build bots didn't see the patch that landed in WebKit. Trying to commit again.
On 2011/08/16 17:04:25, fsamuel wrote: > On 2011/08/13 06:55:32, I haz the power (commit-bot) wrote: > > Change committed as 96694 > > This was reverted because, for whatever reason build bots didn't see the patch > that landed in WebKit. Trying to commit again. The webkit patch had landed, but I suspect that particular revision hadn't been gardened in chromium yet.
On 2011/08/16 17:15:29, sadrul wrote: > On 2011/08/16 17:04:25, fsamuel wrote: > > On 2011/08/13 06:55:32, I haz the power (commit-bot) wrote: > > > Change committed as 96694 > > > > This was reverted because, for whatever reason build bots didn't see the patch > > that landed in WebKit. Trying to commit again. > > The webkit patch had landed, but I suspect that particular revision hadn't been > gardened in chromium yet. Yea, it should be gardened in now, presumably. I'll commit this (or submit another version of the patch), once the cl to fix the TOUCH_UI build gets committed.
On 2011/08/16 17:18:10, fsamuel wrote: > On 2011/08/16 17:15:29, sadrul wrote: > > On 2011/08/16 17:04:25, fsamuel wrote: > > > On 2011/08/13 06:55:32, I haz the power (commit-bot) wrote: > > > > Change committed as 96694 > > > > > > This was reverted because, for whatever reason build bots didn't see the > patch > > > that landed in WebKit. Trying to commit again. > > > > The webkit patch had landed, but I suspect that particular revision hadn't > been > > gardened in chromium yet. > > Yea, it should be gardened in now, presumably. I believe so (look for 'webkit_revision' in src/DEPS in the chromium tree; that tells you the gardened revision). > I'll commit this (or submit > another version of the patch), once the cl to fix the TOUCH_UI build gets > committed. If you want to recommit this (you won't need LG TM again), you can reopen (from Edit Issue) and click the 'Commit' checkbox.
On 2011/08/16 17:18:10, fsamuel wrote: > On 2011/08/16 17:15:29, sadrul wrote: > > On 2011/08/16 17:04:25, fsamuel wrote: > > > On 2011/08/13 06:55:32, I haz the power (commit-bot) wrote: > > > > Change committed as 96694 > > > > > > This was reverted because, for whatever reason build bots didn't see the > patch > > > that landed in WebKit. Trying to commit again. > > > > The webkit patch had landed, but I suspect that particular revision hadn't > been > > gardened in chromium yet. > > Yea, it should be gardened in now, presumably. I'll commit this (or submit > another version of the patch), once the cl to fix the TOUCH_UI build gets > committed. Ohh awesome. Thanks for pointing that out Sadrul!
Change committed as 97052 |
