Last Comment Bug 711966 - Change Tilt button label to "3D View" and the accesskey to "w"
: Change Tilt button label to "3D View" and the accesskey to "w"
Status: RESOLVED FIXED
[tilt]
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 12
Assigned To: Victor Porof [:vporof][:vp]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-19 06:28 PST by Rob Campbell [:rc] (:robcee)
Modified: 2012-04-23 10:46 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (1.52 KB, patch)
2011-12-19 08:24 PST, Victor Porof [:vporof][:vp]
no flags Details | Diff | Splinter Review
v2 (1.64 KB, patch)
2011-12-19 11:12 PST, Victor Porof [:vporof][:vp]
dao+bmo: review-
Details | Diff | Splinter Review
v3 (1.88 KB, patch)
2011-12-24 02:22 PST, Victor Porof [:vporof][:vp]
dao+bmo: review-
Details | Diff | Splinter Review
v4 (1.94 KB, patch)
2011-12-24 03:47 PST, Victor Porof [:vporof][:vp]
dao+bmo: review-
Details | Diff | Splinter Review
v5 (1.08 KB, patch)
2011-12-24 04:06 PST, Victor Porof [:vporof][:vp]
dao+bmo: review-
Details | Diff | Splinter Review
v6 (1.84 KB, patch)
2011-12-24 04:19 PST, Victor Porof [:vporof][:vp]
dao+bmo: review+
Details | Diff | Splinter Review
[in-fx-team] v7 (1.85 KB, patch)
2011-12-24 04:23 PST, Victor Porof [:vporof][:vp]
dao+bmo: review+
rcampbell: review+
Details | Diff | Splinter Review

Description Rob Campbell [:rc] (:robcee) 2011-12-19 06:28:13 PST
Follow up from bug 689920.

The accelerator key for the 3D menu button in the Page Inspector is 'M'. We should change this to the number '3' key for Windows and Linux to match the string.
Comment 1 Victor Porof [:vporof][:vp] 2011-12-19 08:24:37 PST
Created attachment 582836 [details] [diff] [review]
v1

Also changed access key to D on mac.
Comment 2 Marek Stępień [:marcoos, inactive] 2011-12-19 09:52:49 PST
There should be at least a localization note in the DTD file telling localizers that accesskey2 is for Mac...
Comment 3 Victor Porof [:vporof][:vp] 2011-12-19 11:12:37 PST
Created attachment 582888 [details] [diff] [review]
v2

Added localization note.
Comment 4 Dão Gottwald [:dao] 2011-12-19 16:07:58 PST
Comment on attachment 582888 [details] [diff] [review]
v2

Alt+3 selects the third tab on Linux.
Comment 5 Dão Gottwald [:dao] 2011-12-19 16:09:26 PST
Any reason against using D across platforms?
Comment 6 Victor Porof [:vporof][:vp] 2011-12-19 22:48:44 PST
(In reply to Dão Gottwald [:dao] from comment #5)
> Any reason against using D across platforms?

Alt+D focuses the awesomebar on Windows.
Comment 7 Victor Porof [:vporof][:vp] 2011-12-19 22:56:52 PST
(In reply to Dão Gottwald [:dao] from comment #4)
> Comment on attachment 582888 [details] [diff] [review]
> v2
> 
> Alt+3 selects the third tab on Linux.

Maybe we can use 3 on Windows, D on Mac OS X and Linux? I just tested in Windows, Alt+3 doesn't apparently do anything.
Comment 8 Rob Campbell [:rc] (:robcee) 2011-12-21 06:32:50 PST
that could work but it sure is a lot of keys for one button. :)
Comment 9 Victor Porof [:vporof][:vp] 2011-12-24 02:22:23 PST
Created attachment 584169 [details] [diff] [review]
v3
Comment 10 Dão Gottwald [:dao] 2011-12-24 03:35:04 PST
Comment on attachment 584169 [details] [diff] [review]
v3

- use #ifdef XP_WIN
- set inspect3DButton.accesskey to D
- add inspect3DButton.windowsAccesskey with 3
Comment 11 Dão Gottwald [:dao] 2011-12-24 03:40:33 PST
(In reply to Victor Porof from comment #7)
> (In reply to Dão Gottwald [:dao] from comment #4)
> > Comment on attachment 582888 [details] [diff] [review]
> > v2
> > 
> > Alt+3 selects the third tab on Linux.
> 
> Maybe we can use 3 on Windows, D on Mac OS X and Linux? I just tested in
> Windows, Alt+3 doesn't apparently do anything.

Err, doesn't Alt+D focus the location bar on Linux as well?
Comment 12 Victor Porof [:vporof][:vp] 2011-12-24 03:47:44 PST
Created attachment 584175 [details] [diff] [review]
v4

Addressed Dão's review comments.
Comment 13 Dão Gottwald [:dao] 2011-12-24 03:49:14 PST
Comment on attachment 584175 [details] [diff] [review]
v4

-> comment 11 :(
Comment 14 Victor Porof [:vporof][:vp] 2011-12-24 03:53:14 PST
(In reply to Dão Gottwald [:dao] from comment #13)
> Comment on attachment 584175 [details] [diff] [review]
> v4
> 
> -> comment 11 :(

Just saw it. Hmm, yes, it does.
3 on Windows and Linux, D on Mac?
Comment 15 Victor Porof [:vporof][:vp] 2011-12-24 03:55:48 PST
(In reply to Victor Porof from comment #14)
> (In reply to Dão Gottwald [:dao] from comment #13)
> > Comment on attachment 584175 [details] [diff] [review]
> > v4
> > 
> > -> comment 11 :(
> 
> Just saw it. Hmm, yes, it does.
> 3 on Windows and Linux, D on Mac?

Wait, no, comment 4 :(
Any other suggestions?
Comment 16 Dão Gottwald [:dao] 2011-12-24 03:58:57 PST
Label the button "3D View" and use V?
Comment 17 Victor Porof [:vporof][:vp] 2011-12-24 04:00:59 PST
(In reply to Dão Gottwald [:dao] from comment #16)
> Label the button "3D View" and use V?

Or "3D DOM" and continue using M. Which one sounds better?
Comment 18 Dão Gottwald [:dao] 2011-12-24 04:03:54 PST
3D DOM doesn't seem to make sense to me...
Comment 19 Victor Porof [:vporof][:vp] 2011-12-24 04:06:28 PST
Created attachment 584177 [details] [diff] [review]
v5
Comment 20 Victor Porof [:vporof][:vp] 2011-12-24 04:09:45 PST
Alt+V opens the View menu on Linux (probably Windows too).
I think we should WONTFIX this one.
Comment 21 Dão Gottwald [:dao] 2011-12-24 04:11:11 PST
Comment on attachment 584177 [details] [diff] [review]
v5

We need to change the entity name here... Localizers have already picked up 3D on aurora, together with random bogus access keys.
Comment 22 Dão Gottwald [:dao] 2011-12-24 04:12:00 PST
(In reply to Victor Porof from comment #20)
> Alt+V opens the View menu on Linux (probably Windows too).
> I think we should WONTFIX this one.

Alt+W?
Comment 23 Victor Porof [:vporof][:vp] 2011-12-24 04:16:18 PST
(In reply to Dão Gottwald [:dao] from comment #21)
> Comment on attachment 584177 [details] [diff] [review]
> v5
> 
> We need to change the entity name here... Localizers have already picked up
> 3D on aurora, together with random bogus access keys.

Is inspect3DButton.accesskey3d a good entity name?
Comment 24 Dão Gottwald [:dao] 2011-12-24 04:19:22 PST
No, accesskey3d doesn't make sense... You could just call it inspect3DViewButton.accesskey.
Comment 25 Victor Porof [:vporof][:vp] 2011-12-24 04:19:22 PST
Created attachment 584179 [details] [diff] [review]
v6

Found a better solution.
Comment 26 Dão Gottwald [:dao] 2011-12-24 04:21:58 PST
Comment on attachment 584179 [details] [diff] [review]
v6

>+<!-- LOCALIZATION NOTE (inspect3DButton.label): This button shows an alternate
>+  -  view for the Inspector, creating a 3D visualization of the webpage. -->
>+<!ENTITY inspect3DViewButton.label     "3D View">

update the entity reference in the localization note
Comment 27 Dão Gottwald [:dao] 2011-12-24 04:22:47 PST
What can we do to prevent locales from picking access keys that conflict with the menu bar?
Comment 28 Victor Porof [:vporof][:vp] 2011-12-24 04:23:04 PST
Created attachment 584180 [details] [diff] [review]
[in-fx-team] v7

Seventh time the charm :)
Comment 29 Victor Porof [:vporof][:vp] 2011-12-24 04:24:00 PST
(In reply to Dão Gottwald [:dao] from comment #27)
> What can we do to prevent locales from picking access keys that conflict
> with the menu bar?

For Tilt specifically, we could add a test to check if using the access key indeed opens the visualization.
Comment 30 Dão Gottwald [:dao] 2011-12-24 04:26:10 PST
(In reply to Victor Porof from comment #29)
> (In reply to Dão Gottwald [:dao] from comment #27)
> > What can we do to prevent locales from picking access keys that conflict
> > with the menu bar?
> 
> For Tilt specifically, we could add a test to check if using the access key
> indeed opens the visualization.

Well, we'd to run that test on localized builds... The concern also isn't limited to this single button.
Comment 31 Victor Porof [:vporof][:vp] 2011-12-24 10:40:12 PST
(In reply to Dão Gottwald [:dao] from comment #30)
> (In reply to Victor Porof from comment #29)
> > (In reply to Dão Gottwald [:dao] from comment #27)
> > > What can we do to prevent locales from picking access keys that conflict
> > > with the menu bar?
> > 
> > For Tilt specifically, we could add a test to check if using the access key
> > indeed opens the visualization.
> 
> Well, we'd to run that test on localized builds... The concern also isn't
> limited to this single button.

Filed bug 713391 for this.
Comment 32 Rob Campbell [:rc] (:robcee) 2012-01-03 08:50:52 PST
Comment on attachment 584180 [details] [diff] [review]
[in-fx-team] v7

W, eh? Why not!
Comment 33 Victor Porof [:vporof][:vp] 2012-01-13 03:47:37 PST
So can we land this? :)
Comment 34 Rob Campbell [:rc] (:robcee) 2012-01-13 10:46:51 PST
yes. please put [land-in-fx-team] in the status whiteboard when ready to ship.
Comment 35 Mihai Sucan [:msucan] 2012-01-16 09:33:02 PST
Comment on attachment 584180 [details] [diff] [review]
[in-fx-team] v7

Landed:
https://hg.mozilla.org/integration/fx-team/rev/268524cb6639

Quick note: accesskeys are case sensitive. You have "3D View" with "W" which should have been lowercase. (unfortunately, I just noticed after landing)

For future reference, please check: https://developer.mozilla.org/en/XUL_Accesskey_FAQ_and_Policies
Comment 36 Victor Porof [:vporof][:vp] 2012-01-16 09:38:23 PST
Will this make the button text be "3D View (W)"? We can do a quick follow-up.
Comment 37 Mihai Sucan [:msucan] 2012-01-16 09:39:41 PST
(In reply to Victor Porof from comment #36)
> Will this make the button text be "3D View (W)"? We can do a quick follow-up.

I hope it won't.
Comment 38 Victor Porof [:vporof][:vp] 2012-01-16 09:46:34 PST
Filed bug 718458.
Comment 39 Tim Taubert [:ttaubert] 2012-01-17 04:25:53 PST
https://hg.mozilla.org/mozilla-central/rev/268524cb6639
Comment 40 Dão Gottwald [:dao] 2012-01-17 06:11:34 PST
This landed with a nonsensical commit message :/
Comment 41 Rob Campbell [:rc] (:robcee) 2012-01-20 09:59:14 PST
Copy Checkin Comment FTL.
Comment 42 Eric Shepherd [:sheppy] 2012-04-23 10:34:38 PDT
This is already documented as "3D view"
Comment 43 Eric Shepherd [:sheppy] 2012-04-23 10:46:32 PDT
This is already documented as "3D view"

Note You need to log in before you can comment on or make changes to this bug.