1.11 KB, patch
|Details | Diff | Splinter Review|
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
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
Setting assignee as requested on IRC :-)
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.
(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.
(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
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.
Created attachment 531241 [details] [diff] [review] 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).
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 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!
Created attachment 531500 [details] [diff] [review] 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.
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.
Pushed: http://hg.mozilla.org/mozilla-central/rev/106e33ecffb6 Thank you for your contribution Brian :)
Verified on: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110515 Firefox/6.0a1