PDF: Add shortcut to toggle fit-to-{page,width}.

PDF: Add shortcut to toggle fit-to-{page,width}.

ctrl + \

BUG=636515
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b9e8d9e97bd881fff98082b4490483926fa84aed
Cr-Commit-Position: refs/heads/master@{#416876}

[Lei Zhang, 2016-08-26 20:43:23]

Description was changed from

==========
PDF: Add shortcut to toggle fit-to-{page,width}.

ctrl + \

BUG=636515
==========

to

==========
PDF: Add shortcut to toggle fit-to-{page,width}.

ctrl + \

BUG=636515
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Lei Zhang, 2016-08-26 20:44:17]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[Lei Zhang, 2016-08-26 20:44:37]

thestig@chromium.org changed reviewers:
+ raymes@chromium.org

[commit-bot: I haz the power, 2016-08-26 20:45:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 21:50:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 21:50:40]

Dry run: This issue passed the CQ dry run.

[raymes, 2016-08-29 03:35:09]

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/pdf.js:400: if (e.ctrlKey) {
Do you think it might be confusing that this might result in something that is
different to what would happen by clicking the ftw/ftp button? The result of the
button actually depends on the last state of the button as opposed to the
current state of the zoom (I'd be ok if that changed though).

[Lei Zhang, 2016-08-29 05:33:54]

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/pdf.js:400: if (e.ctrlKey) {
On 2016/08/29 03:35:09, raymes wrote:
> Do you think it might be confusing that this might result in something that is
> different to what would happen by clicking the ftw/ftp button? The result of
the
> button actually depends on the last state of the button as opposed to the
> current state of the zoom (I'd be ok if that changed though).

Probably. I'll take a closer look tomorrow.

[Lei Zhang, 2016-09-02 02:15:59]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-02 02:16:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-09-02 02:17:52]

See patch set 2.

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/2283803003/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/pdf.js:400: if (e.ctrlKey) {
On 2016/08/29 05:33:54, Lei Zhang wrote:
> On 2016/08/29 03:35:09, raymes wrote:
> > Do you think it might be confusing that this might result in something that
is
> > different to what would happen by clicking the ftw/ftp button? The result of
> the
> > button actually depends on the last state of the button as opposed to the
> > current state of the zoom (I'd be ok if that changed though).
> 
> Probably. I'll take a closer look tomorrow.

Today is tomorrow somewhere in the world.

[commit-bot: I haz the power, 2016-09-02 02:28:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-02 02:28:19]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[raymes, 2016-09-05 03:21:40]

https://codereview.chromium.org/2283803003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/2283803003/diff/20001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:406:
this.zoomToolbar_.toggleFitButtonUIState();
Could we just call fitToggle on the zoom toolbar? I know it will be calling back
out to here, but maybe it makes the state tracking more simple? WDYT?

[Lei Zhang, 2016-09-07 00:13:21]

Patchset #3 (id:40001) has been deleted

[Lei Zhang, 2016-09-07 00:26:18]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 00:26:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-09-07 00:28:13]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 00:28:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-09-07 00:38:47]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[Lei Zhang, 2016-09-07 00:39:09]

Took a few tries...

https://codereview.chromium.org/2283803003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/2283803003/diff/20001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:406:
this.zoomToolbar_.toggleFitButtonUIState();
On 2016/09/05 03:21:40, raymes wrote:
> Could we just call fitToggle on the zoom toolbar? I know it will be calling
back
> out to here, but maybe it makes the state tracking more simple? WDYT?

Done.

[commit-bot: I haz the power, 2016-09-07 00:39:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 01:04:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 01:04:38]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[raymes, 2016-09-07 05:39:10]

Thanks - we actually do have a test for this element in
material_elements_test.js. Perhaps we can test this function from there? (Sorry
I didn't mention this before, the thought only just came to me that we might
have a test for this).

[Lei Zhang, 2016-09-07 06:27:17]

On 2016/09/07 05:39:10, raymes wrote:
> Thanks - we actually do have a test for this element in
> material_elements_test.js. Perhaps we can test this function from there?
(Sorry
> I didn't mention this before, the thought only just came to me that we might
> have a test for this).

Done.

[Lei Zhang, 2016-09-07 06:27:25]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 06:27:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-09-07 06:28:03]

lgtm thanks!

[commit-bot: I haz the power, 2016-09-07 07:45:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 07:46:00]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-09-07 07:48:52]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-09-07 07:49:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 07:53:07]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-09-07 07:55:00]

Description was changed from

==========
PDF: Add shortcut to toggle fit-to-{page,width}.

ctrl + \

BUG=636515
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
PDF: Add shortcut to toggle fit-to-{page,width}.

ctrl + \

BUG=636515
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b9e8d9e97bd881fff98082b4490483926fa84aed
Cr-Commit-Position: refs/heads/master@{#416876}
==========

[commit-bot: I haz the power, 2016-09-07 07:55:02]

Patchset 7 (id:??) landed as
https://crrev.com/b9e8d9e97bd881fff98082b4490483926fa84aed
Cr-Commit-Position: refs/heads/master@{#416876}