Closed Bug 1260235 Opened 4 years ago Closed 4 years ago

cmd+f doesn't focus the inspector search box on osx

Categories

(DevTools :: Inspector, defect, P1)

46 Branch
defect

Tracking

(firefox46 unaffected, firefox47 verified, firefox48 verified)

VERIFIED FIXED
Firefox 48
Tracking Status
firefox46 --- unaffected
firefox47 --- verified
firefox48 --- verified

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

(Keywords: regression, Whiteboard: [btpp-fix-now])

Attachments

(2 files)

STR:

Open inspector
Press cmd+f

Expected:

Search box is focused

Actual:

It's not focused
Seems to work for me in beta but not Dev Edition
As far as I can tell: Bug 1238133 made cmd+f start to focus the rule view only.  And then (somehow) the CSS change in Bug 1243598 seemed to make neither get focused.  At least, that looks like the only relevant change in https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=709f559b5406e8555cf84dd09bdc747b076f142c&tochange=3b35f0a98eba9f1c9be7d793650b3d5bec6c8fdb.

So, we need to be able to handle multiple instances of ctrl/cmd+f inside of a single frame.  I think what's actually happening is that the computed view's search box is getting focused when cmd+f is pressed.
Blocks: 1238133, 1243598
WIP - testing this is proving hard since events sent from EventUtils.synthesizeKey doesn't seem to behave the same way as ones sent normally (i.e. event.code and event.keyCode aren't set).
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
I don't love the solution, especially since it's hard to focus elements in the rule view (at least on osx if I click on various spots in the rule view I can make cmd+f focus the inspector search).   But it's much better than the current behavior IMO.

This might be a real reason to push some of this stuff back into iframes but keep a lot of the refactoring done originally to de-frame it (maybe one frame for all sidebar tabs now that we have a layout fix in Bug 1243598).

Anyway, try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcac194ff650
Priority: -- → P1
Whiteboard: [btpp-fix-now]
Comment on attachment 8735660 [details]
MozReview Request: Bug 1260235 - Only accept ctrl+f in computed and rule view if it happened in the relevant container;r=gl

https://reviewboard.mozilla.org/r/42899/#review39593
Attachment #8735660 - Flags: review?(gl) → review+
https://hg.mozilla.org/mozilla-central/rev/2b4d5580c46d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
See Also: → 1263104
Hi Brian, this seems to be a recent regression in Fx47. Should we consider uplifting this to Beta47?
Flags: needinfo?(bgrinstead)
(In reply to Ritu Kothari (:ritu) from comment #10)
> Hi Brian, this seems to be a recent regression in Fx47. Should we consider
> uplifting this to Beta47?

Sure, seems simple enough - I'll file a request
Flags: needinfo?(bgrinstead)
Comment on attachment 8735660 [details]
MozReview Request: Bug 1260235 - Only accept ctrl+f in computed and rule view if it happened in the relevant container;r=gl

Approval Request Comment
[Feature/regressing bug #]: 1243598
[User impact if declined]: Ctrl/Cmd+F in the devtools inspector won't focus the search box
[Describe test coverage new/current, TreeHerder]: Regression test added for this
[Risks and why]: This is dealing with keyboard handling.  The worst case is that it would break keyboard shortcuts in the inspector panel.  But it's not doing much - basically early returning in a couple of places so that the 'side panels' don't take over key events when they shouldn't: https://hg.mozilla.org/mozilla-central/rev/2b4d5580c46d#l1.12 and https://hg.mozilla.org/mozilla-central/rev/2b4d5580c46d#l3.12
[String/UUID change made/needed]: none
Attachment #8735660 - Flags: approval-mozilla-beta?
I've done manual testing on a beta build and this applied cleanly / fixes the problem
Hi Florin, I am planning to take this fix in 47.0b2. Could you please do some focused testing on keyboard shortcuts in case there are any unexpected regressions from this change? Thanks!
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
Comment on attachment 8735660 [details]
MozReview Request: Bug 1260235 - Only accept ctrl+f in computed and rule view if it happened in the relevant container;r=gl

Recent P1 regression, Beta47+
Attachment #8735660 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ritu Kothari (:ritu) from comment #14)
> Hi Florin, I am planning to take this fix in 47.0b2. Could you please do
> some focused testing on keyboard shortcuts in case there are any unexpected
> regressions from this change? Thanks!

Setting ni? for Andrei Vaida. This was included in the 47 Beta 2 sign-offs (https://goo.gl/jhXbqn). Will follow up with results once done.
Flags: needinfo?(florin.mezei) → needinfo?(andrei.vaida)
Verified this issue as fixed on Windows XP x86, Windows 8.1 x64, Mac OS X 10.10.4 and Ubuntu 13.04 x86 using Fx 47 beta 2.
No new issues were found while performing exploratory testing around the keyboard shortcuts.
Flags: needinfo?(andrei.vaida)
Depends on: 1278552
Verified as fixed on Windows XP x64, Windows 10 x64, Mac OS X 10.11.1 and Ubuntu 14.04 x64 using Fx 48 beta 2 (20160620091522).
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.