Virgil Dicu [:virgil] [QA] Reporter
	Description • 11 years ago

Both zoom buttons don't perform any action and according to bug 801129 comment 3 those buttons should be removed. 

Can't see any other bug logged for this.

	Brian R. Bondy [:bbondy]
	Comment 1 • 11 years ago

p=1

	Marco Mucci [:MarcoM]
	Comment 2 • 11 years ago

Hi Ally, are you planning on taking the bug for IT#8?

	Allison Naaktgeboren :ally Assignee
	Comment 3 • 11 years ago

sure.

	Allison Naaktgeboren :ally Assignee
	Comment 4 • 11 years ago

- I went for surgical removal; I want the feature to be inaccessible in the ui, but I know we want it back fro v2, so I don't want to rip out core code that we'll use later, so I removed only ui code & its immediate touch points.
- The images for the icons are part of one giant file, so I left that alone
- I did remove them from the list of supported commands in browser-ui

	Allison Naaktgeboren :ally Assignee
	Comment 6 • 11 years ago

Comment on attachment 756774 [details] [diff] [review]
zoom buttons removed

I think I might have missed something. pulling review request

	Allison Naaktgeboren :ally Assignee
	Comment 7 • 11 years ago

forgot to qref that. See Notes on previous patch

	Matt Brubeck (:mbrubeck)
	Comment 8 • 11 years ago

Comment on attachment 756778 [details] [diff] [review]
zoom buttons, keyboard shortcuts for them removed

Review of attachment 756778 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/browser.xul
@@ -143,5 @@
>      <key id="key_save" key="s" modifiers="accel" command="cmd_savePage"/>
>  
>      <!-- misc -->
> -    <key id="key_zoomin" key="+" modifiers="accel" command="cmd_zoomin"/>
> -    <key id="key_zoomout" key="-" modifiers="accel" command="cmd_zoomout"/>

We'll probably want to add these shortcuts back at some point, but if they're broken right now, let's go ahead and remove them.

	Allison Naaktgeboren :ally Assignee
	Comment 9 • 11 years ago

yea, my thought was to add them back when we had the ui back. In my perfect world, the keyboard access perfectly mirrors the gui access.

	Matt Brubeck (:mbrubeck)
	Comment 10 • 11 years ago

Comment on attachment 756778 [details] [diff] [review]
zoom buttons, keyboard shortcuts for them removed

Review of attachment 756778 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/appbar.js
@@ -63,3 @@
>          break;
>        case 'MozPrecisePointer':
>        case 'MozImprecisePointer':

Almost forgot - please remove this entire (now-empty) "case:" section and the corresponding addEventListener calls.

	Phil Ringnalda (:philor)
	Comment 12 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ce9af0215ac

	Phil Ringnalda (:philor)
	Comment 13 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/0ce9af0215ac

	Mihaela Velimiroviciu (:mihaelav)
	Comment 14 • 11 years ago

Mozilla/5.0 (Windows NT 6.2; rv:24.0) Gecko/20130610 Firefox/24.0

Verified that the zoom buttons were removed from the navigation bar. Also, CTRL + +/- don't zoom in/out, but CTRL + mouse scroll does zoom in/out. Is this intended?

	Kamil Jozwiak [:kjozwiak]
	Comment 15 • 11 years ago

Went through the following "Change" for iteration #9 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-25-03-12-38-mozilla-central/

- Ensured that the zoom buttons have been removed from the App Bar (using both touch & keyboard/mouse)
- Ensured that using CTRL + +/- doesn't zoom in
- Ensured that tapping/clicking on App Bar didn't enable any buttons that have been hidden
- Ensured that the above cases also worked in Filled View without any issues

Notes:

- As mentioned in comment 14, pressing CTRL + Mouse Scroll will still zoom in/out. Asa, before marking this as verified, should that also be removed?

	Asa Dotzler [:asa]
	Comment 16 • 11 years ago

(In reply to Kamil Jozwiak [:kjozwiak] from comment #15)
> Went through the following "Change" for iteration #9 testing without any
> issues. Used the following build:
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-25-03-12-38-
> mozilla-central/
> 
> - Ensured that the zoom buttons have been removed from the App Bar (using
> both touch & keyboard/mouse)
> - Ensured that using CTRL + +/- doesn't zoom in
> - Ensured that tapping/clicking on App Bar didn't enable any buttons that
> have been hidden
> - Ensured that the above cases also worked in Filled View without any issues
> 
> Notes:
> 
> - As mentioned in comment 14, pressing CTRL + Mouse Scroll will still zoom
> in/out. Asa, before marking this as verified, should that also be removed?

Let's not worry about the legacy ctrl+mousewheel thing for now. I suspect our APZC stuff is going to simply replace or break all that.

	Samvedana (:Samvedana)
	Comment 17 • 11 years ago

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130816030205
Built from http://hg.mozilla.org/mozilla-central/rev/1ed5a88cd4d0

WFM
Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment15 and got expected result.

- Ensured that the zoom buttons have been removed from the App Bar (using both touch & keyboard/mouse)
- Ensured that using CTRL + +/- doesn't zoom in
- Ensured that tapping/clicking on App Bar didn't enable any buttons that have been hidden
- Ensured that the above cases also worked in Filled View without any issues

Notes:
- CTRL + Mouse Scroll will still zoom in/out.

	Manuela Muntean [Away]
	Comment 18 • 11 years ago

WFM with the latest Nightly (build ID: 20131103030204) on Win 8 32-bit, while testing for iteration #17.

Notes:
- CTRL + Mouse Scroll will still zoom in/out