Closed Bug 815273 Opened 12 years ago Closed 12 years ago

Focus ring immediately after opening

Categories

(Firefox :: Downloads Panel, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 20

People

(Reporter: evilpie, Assigned: mconley)

Details

Attachments

(2 files, 1 obsolete file)

Attached image Screengrab
Even when I just open the Download Panel by clicking on the button in the bar I always get this dotted line around Show All Downloads. No keyboard or anything tricky involved (as far as I can tell).
On Windows I can see the same if I toggle with the keyboard through the toolbox and then open the button with the mouse (though may be intended since I inited a keyboard navigation somehow).

An interesting thing is that gnomestripe global button.css seems to use :focus instead of -moz-focusring, for some reason pinstripe and winstripe were updated in bug 418521, but not gnomestripe.
So, we force focus on the button here:

http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/content/downloads.js?rev=a3019cbb0945#359

This is so that when the user hits the "paste" key combination, the keydown event is "seen" by the panel.

I just tried setting focus on the panel itself, instead of the button - but for some reason, at least with gtk2, it complains that the panel is already focused...but then key events don't seem to reach the panel.

I tested this with the Bookmarks inline editing panel as well - I disabled it's auto focus/select of the input elements, and got it to focus the panel itself. No key events were received by the panel.

This appears to be true for both Windows and Gtk2. I haven't tried OSX yet.

Enn - do you know if this is somehow by design?
Flags: needinfo?(enndeakin)
(In reply to Mike Conley (:mconley) from comment #2)
> So, we force focus on the button here:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/
> content/downloads.js?rev=a3019cbb0945#359
> 
> This is so that when the user hits the "paste" key combination, the keydown
> event is "seen" by the panel.
> 

If the link is focused, pressing the paste keyboard shortcut should paste into the link. From the screenshot, that doesn't seem like a sensible action.
Flags: needinfo?(enndeakin)
(In reply to Neil Deakin from comment #3)
> (In reply to Mike Conley (:mconley) from comment #2)
> > So, we force focus on the button here:
> > 
> > http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/
> > content/downloads.js?rev=a3019cbb0945#359
> > 
> > This is so that when the user hits the "paste" key combination, the keydown
> > event is "seen" by the panel.
> > 
> 
> If the link is focused, pressing the paste keyboard shortcut should paste
> into the link. From the screenshot, that doesn't seem like a sensible action.

Right - this is a workaround, because if we only focus the panel (as opposed to a child of the panel), then the panel doesn't seem to react to keypress events.

Is that by design?
Flags: needinfo?(enndeakin)
So it turns out that I can set -moz-user-focus: normal on the panel, which allows us to focus it, which allows the keyevents to go through.

Patch up in a few minutes.
Flags: needinfo?(enndeakin)
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → mconley
Comment on attachment 685646 [details] [diff] [review]
Patch v1

Ok, we focus the panel now instead of the button. No focusring!

\o/
Attachment #685646 - Flags: review?(mak77)
Comment on attachment 685646 [details] [diff] [review]
Patch v1

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

My fear is that we are going to change the default panel focus behavior, so I'm totally unsure which focus bugs we may uncover.
Let'try it, we still have quite some time to test it in Nightly and Aurora, in the worst case a backout will just cause a polish issue to come back and can evaluate another path.

::: browser/themes/gnomestripe/downloads/downloads.css
@@ +5,5 @@
>  /*** Panel and outer controls ***/
>  
> +#downloadsPanel {
> +  -moz-user-focus: normal;
> +}

This is a functionality rule, thus should go to browser/components/downloads/content/downloads.js, not to themes
Attachment #685646 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #8)
> Comment on attachment 685646 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 685646 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> My fear is that we are going to change the default panel focus behavior, so
> I'm totally unsure which focus bugs we may uncover.
> Let'try it, we still have quite some time to test it in Nightly and Aurora,
> in the worst case a backout will just cause a polish issue to come back and
> can evaluate another path.

Coolbeans.

> 
> ::: browser/themes/gnomestripe/downloads/downloads.css
> @@ +5,5 @@
> >  /*** Panel and outer controls ***/
> >  
> > +#downloadsPanel {
> > +  -moz-user-focus: normal;
> > +}
> 
> This is a functionality rule, thus should go to
> browser/components/downloads/content/downloads.js, not to themes

Correct - thanks for catching that! (I assume you meant b/c/downloads/content/downloads.css)
yes sorry, downloads.css
Moved the -moz-user-focus rule to browser/components/downloads/content/downloads.css
Attachment #685646 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/8a84d0ad94c1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Verified as fixed on the latest Nightly - the focus ring is not present anymore in the downloads panel immediately after opening it.

Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121203 Firefox/20.0
Build ID: 20121203030801
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: