652363 - Dragging selection into location bar doesn't set GO button immediately

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[u336495]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110422 Firefox/6.0a1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110422 Firefox/6.0a1

When You drag some selected text into location bar, usually You want to search immediately but GO button does not appear. I still see Reload button. I have to focus location bar to see GO button but it should appear immediately after releasing mouse button on location bar.

Reproducible: Always

Steps to Reproduce:
1.Select some text
2.Drag it to location bar


Actual Results:  
Reload is visible

Expected Results:  
GO button should appear

Comment 1

[Alice0775 White]

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/03f80b4e0318
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100902 Firefox/4.0b6pre ID:20100902180724
Fails:
http://hg.mozilla.org/mozilla-central/rev/7aaf30721c48
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100902 Firefox/4.0b6pre ID:20100902204148
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=03f80b4e0318&tochange=7aaf30721c48

Comment 2

[Ed Morley [:emorley]]

Setting assignee as requested on IRC :-)

Comment 3

[Brian R. Bondy [:bbondy]]

This bug is actually a little more generalized than reported, the reported bug is a subset of the generalized problem though.

The more generalized problem:

Reproducible: Always

Steps to Reproduce:
1. Enter some text in the location bar
2. click somewhere on the currently loaded HTML page to take focus away from the location bar.  

Actual results: 
The reload button appears once focus is taken away

Expected results: 
The GO button should stay once focus is taken away

Older versions of Firefox 3.* worked in the way described in my "Expected results" above.  That is to say the GO button would stay whether there was focus or not.

The drag and drop problem reported is a subset of this bug because after you drag and drop text on the location bar, the location bar does not have focus.  If you then put focus in the location bar, then the GO button will appear. 

To fix this problem I will be making the GO button appear whether there is focus or not on the location bar if there is something new to load.  This will fix the drag and drop problem reported and the more generalized problem as described in this post. 

An alternate way to fix would be to give the location bar focus when dropping text on it.  But then if focus was taken away the reload button would re-appear.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> To fix this problem I will be making the GO button appear whether there is
> focus or not on the location bar if there is something new to load.

That makes sense to me. I don't know why bug 544816 changed this behavior.

Comment 5

[Frank Yan (:fryn)]

(In reply to comment #4)
> (In reply to comment #3)
> > To fix this problem I will be making the GO button appear whether there is
> > focus or not on the location bar if there is something new to load.
> 
> That makes sense to me. I don't know why bug 544816 changed this behavior.

I intentionally didn't do this in bug 544816, because while the location bar isn't focused, we guess that a user is more likely to want to reload the page.

For example, if a user types something into the url bar, then unfocuses the url bar, and later wants to reload the page, is it reasonable for the user to have to focus the url bar, press escape, and then click the reload button? I don't think so.

However, if we determine that we'd rather have this fix, the code that needs to be changed is pure CSS here:
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.css#251

(In reply to comment #3)
> An alternate way to fix would be to give the location bar focus when
> dropping text on it.

I advocate for this fix. I think the code that needs to be changed is here:
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#431

Comment 6

[Brian R. Bondy [:bbondy]]

Thanks fryn, I identified both places for both potential fixes as well.

I'll submit a patch for the alternate way I suggested that you advocate for.  

Thanks for the explanation on the urlbar focus/GO button, the argument is sound to me.

Comment 7

[Brian R. Bondy [:bbondy]]

Attachment: Patch for showing the GO button when dragging text on the location bar

This patch is the implementation of the proposed alternate fix suggestion I gave in Comment 3. Frank Yan [:fryn] gives justification for this method in Comment 5 (https://bugzilla.mozilla.org/show_bug.cgi?id=652363#c5).

The patch fixes the originally posted bug, and not the generalized problem as I described it (which fryn explains is not a problem in #c5).

Comment 8

[u336495]

As Brian asked me personally to give my opinion, I think that fixing described problem and leaving generalized one unchanged is good for me. I agree to example given in #c5 
Thanks for Your work

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 531241 [details] [diff] [review]
Patch for showing the GO button when dragging text on the location bar

I would omit the comment - it's overly specific to this bug. Focusing the URL bar on drops makes sense in general, so no need to explain why (and the reasoning can be fairly easily deduced using HG blame with a bug reference, if it does come up).

r=me with that change. Thanks for the patch!

Comment 10

[Brian R. Bondy [:bbondy]]

Attachment: Removed comment explaining why focus is set on drop

Thanks for the quick review Gavin. I removed the comment explaining why focus is set on drop and attached a new patch.

Comment 11

[Mounir Lamouri (:mounir)]

The patch has been pushed to cedar repository. It will be sync with mozilla-central soon and the bug will be marked as FIXED at this moment.

Comment 12

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/106e33ecffb6

Thank you for your contribution Brian :)

Comment 13

[Simona Badau, Desktop QA]

Verified on:
Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110515 Firefox/6.0a1