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

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools
P2
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: arni2033, Assigned: yzen)

Tracking

({regression})

Trunk
Firefox 53
regression
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox51 wontfix, firefox52 verified, firefox53 verified, firefox-esr45 affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
str
>>>   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.
(Reporter)

Updated

5 months ago
No longer blocks: 1277113
(Reporter)

Updated

5 months ago
Component: Untriaged → Developer Tools
It seems the toolbox grabs focus and eats the back / forward page navigation shortcuts.

Maybe one of :bgrins / :yzen can help?
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox-esr45: --- → affected
Flags: needinfo?(yzenevich)
Flags: needinfo?(bgrinstead)
Priority: -- → P2
(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?
Flags: needinfo?(bgrinstead)
(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.
Flags: needinfo?(yzenevich)
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
Created attachment 8827959 [details] [diff] [review]
1327972 patch
Attachment #8827959 - Flags: review?(jryans)
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!
Attachment #8827959 - Flags: review?(jryans) → review+

Comment 6

4 months ago
Pushed by yura.zenevich@gmail.com:
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
https://hg.mozilla.org/mozilla-central/rev/d6f80f3a2400
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
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
QA Whiteboard: [bugday-20170118]
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(yzenevich)
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
Flags: needinfo?(yzenevich)
Attachment #8827959 - Flags: approval-mozilla-aurora?
Comment on attachment 8827959 [details] [diff] [review]
1327972 patch

let navigation shortcuts work with devtools focused, beta52+
Attachment #8827959 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+

Comment 12

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/07692c9040cf
status-firefox52: affected → fixed
Flags: qe-verify+

Comment 13

4 months ago
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

4 months ago
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]
QA Whiteboard: [bugday-20170118] → [bugday-20170118], [testday-20170203]
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
You need to log in before you can comment on or make changes to this bug.