1327972 - Alt+Left/Right doesn't navigate in tab history if I clicked a button in devtools toolbar

Status: VERIFIED FIXED

(DevTools :: General, defect, P2)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open new tab. Focus urlbar, type "data:,a", press Enter. Focus urlbar, type "data:,b", press Enter.
2. Open devtools
3. Click on button "Responsive design mode" in devtools toolbar. Click the same button again
4. Press Alt+Left

AR:  No visible action
ER:  Browser should navigate to "data:,a"

This is regression from bug 1242852. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5d450730e3e29949ae9556bec510c0e009e10262&tochange=43a78545f93f6eabbf4b5b581bd3790915f0c1a1@ Yura Zenevich [:yzen]:
It seems that this is a regresion caused by your change. Please have a look.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It seems the toolbox grabs focus and eats the back / forward page navigation shortcuts.

Maybe one of :bgrins / :yzen can help?

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> It seems the toolbox grabs focus and eats the back / forward page navigation
> shortcuts.
> 
> Maybe one of :bgrins / :yzen can help?

If we are intending to only take the left/right keys then I think we should skip handing when alt/cmd is held to avoid this. Yura, what's the intended behavior with left/right here?

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> > It seems the toolbox grabs focus and eats the back / forward page navigation
> > shortcuts.
> > 
> > Maybe one of :bgrins / :yzen can help?
> 
> If we are intending to only take the left/right keys then I think we should
> skip handing when alt/cmd is held to avoid this. Yura, what's the intended
> behavior with left/right here?

Yeah I agree, left/right (with no opt keys) allows for keyboard navigation in toolbar so we should bail if alt/cmd keys are used when arrows are handled.

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1327972 patch

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8827959 [details] [diff] [review]
1327972 patch

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

Looks good to me, thanks for working on it!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6f80f3a2400
make sure alt/cmd+lefr/right navigate history even if devtools toolbar is focused. r=jryans

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d6f80f3a2400

Comment 8

[Nazir Ahmed Sabbir [:NaSb]]

Successfully managed to reproduce this bug on Nightly 49.0a1 (2016-05-26) (Build ID: 20160526030223) by the following Comment 0's instruction!

This issue is now verified as fixed on Latest Firefox Nightly 53.0a1 (2017-01-19) (64-bit)

Build ID: 20170119222621
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
OS: Linux 4.4.0-57-generic; elementary OS 0.4

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this when you get a chance.

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8827959 [details] [diff] [review]
1327972 patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1242852
[User impact if declined]: keyboard shortcuts for history navigation won't work if use is also focused on the dev tools toolbar.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]:  yes, as described in Description.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: filter keyboard events if there are modifiers present, so not too complicated
[String changes made/needed]: none

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8827959 [details] [diff] [review]
1327972 patch

let navigation shortcuts work with devtools focused, beta52+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/07692c9040cf

Comment 13

[Md. Majedul islam]

I have reproduced this bug with Nightly 49.0a1 (2016-05-26) (64-bit) on ubuntu 16.10 , 64 Bit !

This bug's fix is verified with latest Beta & Aurora!
Aurora:
Build   ID    	20170131084239
User Agent      Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
 Beta:  
Build ID        20170130065342                                       
 User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

[bugday-20170201]

Comment 14

[Tanvir Rahman]

I have reproduced this bug with Nightly 53.0a1 (2017-01-01) (64-bit)

The Bug's fix is now verified on latest Beta and Aurora

Aurora:
Build ID 	20170205004004
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

Beta:
Build ID 	20170130065342
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0

[testday-20170203]