Closed Bug 1192378 Opened 9 years ago Closed 7 years ago

Arrow icon (Go icon) on the location bar should not look active while location bar is empty

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: Towkir, Assigned: Towkir, Mentored)

Details

Attachments

(1 file)

Attached image Location_bar_arrow.jpg
The arrow icon on the very right side of the location bar is always the same.
I think it should be dead (coloured very light gray and should have no effect while hovering on it) while location bar contains nothing or unless I type something (even a single letter)

The arrow could become alive (active) while I start typing. (It can be active from the frist letter I type)

See attachment to be more clear about this
Hi Gijs, what's your opinion on this ? I know its a minor one, still I want to fix this as the fix will take no time :)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to [:Towkir] Ahmed from comment #2)
> Hi Gijs, what's your opinion on this ? I know its a minor one, still I want
> to fix this as the fix will take no time :)

I'm not the right person to decide if this makes sense, so forwarding the question to shorlander. I'll warn you that fixing *anything* takes time - you have to write a patch, you need to test it, someone needs to review it, etc. etc. :-)
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(shorlander)
(In reply to :Gijs Kruitbosch from comment #3)
> I'll warn you that fixing *anything* takes time -
> you have to write a patch, you need to test it, someone needs to review it, etc. etc. :-)
By 'no time' I meant very small time ;)
I agree. It's weird that it appears to be active when it doesn't do anything. It should get the same disabled treatment we give other icons until it's usable.
Flags: needinfo?(shorlander)
I can work on this if anyone of you can act as a mentor (or find someone else) and tell me where to start.
Status: UNCONFIRMED → NEW
Ever confirmed: true
http://searchfox.org/mozilla-central/source/browser/themes/linux/browser.css#1254

and any other styles for #urlbar-go-button.

It feels like you should make sure the button gets disabled/enabled depending on the value of the url bar. Then make sure it gets styled correctly (possibly by setting opacity to some value less than 1) and ensuring it has no hover/active state when disabled.
Mentor: gijskruitbosch+bugs
how about finding out the textbox in the urlbar is empty or not and then : (conceptual format)
#urlbar-go-button:hover #urlbar:not[empty] {
  -moz-image-region: rect(14px, 42px, 28px, 28px);
}
(In reply to [:Towkir] Ahmed from comment #8)
> how about finding out the textbox in the urlbar is empty or not and then :
> (conceptual format)
> #urlbar-go-button:hover #urlbar:not[empty] {

This selector doesn't make sense - the go button is a descendant of the urlbar, not the other way around.

>   -moz-image-region: rect(14px, 42px, 28px, 28px);
> }

The hover state in that image doesn't strike me as being any lighter, at least on OS X.
(In reply to :Gijs Kruitbosch from comment #7) 
> and any other styles for #urlbar-go-button.
> It feels like you should make sure the button gets disabled/enabled depending on the value of the url > bar. 
 is it possible to disable the urlbar-go-button via css only ? or JS required ?
> Then make sure it gets styled correctly (possibly by setting opacity to some value less than 1) and
> ensuring it has no hover/active state when disabled.

Should it be disabled or hidden (like opacity:0;) when there is nothing on the urlbar ?
(In reply to [:Towkir] Ahmed from comment #10)
> (In reply to :Gijs Kruitbosch from comment #7) 
> > and any other styles for #urlbar-go-button.
> > It feels like you should make sure the button gets disabled/enabled depending on the value of the url > bar. 
>  is it possible to disable the urlbar-go-button via css only ? or JS
> required ?

You would need JS, I think, because the button isn't in a position relative to the input that you could style the button based on the input's state.

> > Then make sure it gets styled correctly (possibly by setting opacity to some value less than 1) and
> > ensuring it has no hover/active state when disabled.
> 
> Should it be disabled or hidden (like opacity:0;) when there is nothing on
> the urlbar ?

Disabled (so not *completely* hidden, more like opacity:0.4 or whatever), to prevent having to deal with the state of the separator and/or the rest of the buttons in the URL bar jumping around when you swap between 1 and 0 characters in the URL bar.
I don't think we have an empty attribute set for XUL textbox, so you would need JS to set that yourself.

Once the attribute is set, if you would like to style the go button based on the URL bar attributes, you can:
- if the go button is one of the next siblings of the URL bar, use:
#urlbar[empty] + #urlbar-go-button (direct sibling)
#urlbar[empty] ~ #urlbar-go-button (indirect sibling)

- if it's the child of the URL bar, use:
#urlbar[empty] #urlbar-go-button

- if it's one of the previous siblings of the URL bar, you'd have to use JS to set an attribute on the button
Hi Tim, 
Thanks for your suggestion, I have found the selector, now its about styling to set and there is another confusion.
The #urlbar-go-button is a child of the #urlbar , so the selector should be :
#urlbar:[value=""] #urlbar-go-button:hover {
    /* my style for disabling the button */
}

but this css won't be updated realtime while user is typing or deleting the characters on urlbar, right ?
Looks like css is not enough here. What do you think ?
Flags: needinfo?(ntim.bugs)
(In reply to [:Towkir] Ahmed from comment #13)
> Hi Tim, 
> Thanks for your suggestion, I have found the selector, now its about styling
> to set and there is another confusion.
> The #urlbar-go-button is a child of the #urlbar , so the selector should be :
> #urlbar:[value=""] #urlbar-go-button:hover {
>     /* my style for disabling the button */
> }
> 
> but this css won't be updated realtime while user is typing or deleting the
> characters on urlbar, right ?
> Looks like css is not enough here. What do you think ?

Tim and I both already said you will need some JS to fix this.
Flags: needinfo?(ntim.bugs)
I am currently a little busy, but I will get back soon on this.
Assignee: nobody → 3ugzilla
Status: NEW → ASSIGNED
(In reply to [:Towkir] Ahmed from comment #15)
> I am currently a little busy, but I will get back soon on this.

Ahmed, are you still looking at this?
Flags: needinfo?(3ugzilla)
Hi Gijs,
I had a look in the meantime, But I think I need more specific steps on this, (as I was not sure where to put the JS code and should it be pure JS or React)
I can leave this open for others if you say so, or may be can fix if I get more specific step :)
Thanks
Flags: needinfo?(3ugzilla)
(In reply to [:Towkir] Ahmed from comment #17)
> Hi Gijs,
> I had a look in the meantime, But I think I need more specific steps on
> this, (as I was not sure where to put the JS code and should it be pure JS
> or React)
> I can leave this open for others if you say so, or may be can fix if I get
> more specific step :)
> Thanks

We don't use react in the browser frontend as yet, so it will need to be plain JS.

I *think* you can set an attribute based on the value in the onInput handler in the URL bar binding's XBL file:

https://dxr.mozilla.org/mozilla-central/rev/2963cf6be7f830c0d2155e2968cfc53585868a76/browser/base/content/urlbarBindings.xml#1075

I would expect that to fire if the user changes it, but once we have a patch you should get ::mak to review it to make sure there isn't an edgecase where we set it elsewhere.

Does that help?
Flags: needinfo?(3ugzilla)
Hi Gijs,
I hope I will be able to create a patch now, and I will let you know if I need anything else meanwhile.
Thanks
Flags: needinfo?(3ugzilla)
Closing as WFM, the design of firefox has changed meanwhile and the icon behavior too.
Hence, this is no longer reproducible.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: