Closed Bug 304198 Opened 19 years ago Closed 15 years ago

Deleted URL from location bar reappears after switching tabs

Categories

(Firefox :: Address Bar, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: jason_quinn, Assigned: leftturnsolutions)

References

Details

(Keywords: verified1.9.1)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

If a URL is deleted from the address bar and the user switches to another tab
and then back to the original tab, the previously removed URL has reappeared in
the address bar. This makes high-lighting and dropping in a new URL from some
tab (for example if it wasn't hyperlinked) into a different specific tab --
which is a common browsing event -- more difficult than it should be.

Reproducible: Always

Steps to Reproduce:
1. Have two or more tabs open and go to some site.
2. Manually delete the URL from some tab.
3. Switch tabs and go back to the original tab.
Actual Results:  
The URL is back!

Expected Results:  
The URL should not have reappeared.
No, deleting the url in the addressbar does not delete it from history.
Jason, do you still see this as a problem when dragging URLs with the current versions of Firefox? If not, please close this bug as WORKSFORME. Thanks!
Whiteboard: CLOSEME 08/21
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072605 Minefield/3.0a7pre

This is pretty easy to reproduce, and I think it's done on purpose. Whether or not it's the correct behavior or not is a different story.
Status: UNCONFIRMED → NEW
Component: Tabbed Browser → Location Bar and Autocomplete
Ever confirmed: true
OS: Linux → All
QA Contact: tabbed.browser → location.bar
Hardware: PC → All
Summary: deleted url from address bar reappears after switching tabs → Deleted URL from location bar reappears after switching tabs
Whiteboard: CLOSEME 08/21
Version: unspecified → Trunk
Adam, I noticed a new aspect of the URL behavior that inclines me to think it is actually a bug (or at least an inconsistency). If you delete just part of a URL and switch tabs, when you come back to the original tab, the URL only has the text that wasn't deleted. By extension, when you delete the whole URL, the whole URL should stay deleted. I just tried this in Gran Paradiso Alpha 8; the behavior there is the same as in FF2.
This patch adds a value !== '' check to browser.js, which prevents a url that the user deleted from the location bar from being overwritten during tab changes.
Attachment #379519 - Flags: review?(michaelkohler)
Comment on attachment 379519 [details] [diff] [review]
Fix to prevent deleted urls from being overwritten during tab change

Why did you assign the patch review to me? Assigning it to nobody since I'm not a reviewer.
Attachment #379519 - Flags: review?(michaelkohler) → review?
Attachment #379519 - Flags: review? → review?(dao)
Comment on attachment 379519 [details] [diff] [review]
Fix to prevent deleted urls from being overwritten during tab change

Dao is a better reviewer for browser
Comment on attachment 379519 [details] [diff] [review]
Fix to prevent deleted urls from being overwritten during tab change

>+  // Don't replace the uri in the location bar if the user deletes it. The 
>+  // initial check for !value was letting a '' string get through, causing 
>+  // the deleted uri to reappear (bug 304198).

That comment is quite wordy... I'd rather remove it altogether and write a browser chrome test for this.

>-  if (!value) {
>+  if (!value && value !== '') {

make this just "if (value == null) {".
Attachment #379519 - Flags: review?(dao) → review+
Removed verbose comments, changed if condition to value == null, added browser chrome test case and updated test/Makefile.in. Test cases will fail without the patch applied to browser.js, and simulates 3 tabs: one with a full url, one with a partially deleted url and one with the url deleted. The tabs are then switched around and the gURLBar.value is checked to make sure the correct url is being displayed.
Attachment #379519 - Attachment is obsolete: true
That's great, thanks. The test is fine as-is, but generally it's better to avoid implementation details like gBrowser.userTypedValue and instead simulate what the user is doing, like here:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_ctrlTab.js#94
In this case, you'd just focus the location bar and synthesize backspace (VK_BACK_SPACE) a few times. Do you want to give that a try?
(In reply to comment #10)
> In this case, you'd just focus the location bar and synthesize backspace
> (VK_BACK_SPACE) a few times.

But note that focusing the location bar will select the address on Windows and Mac, but not on Linux. So you'd have to modify gURLBar.clickSelectsAll, or explicitly clear the selection or explicitly select everything after focusing, in order to get consistent results across platforms.
Thanks again for the feedback Dão. I'll update the test accordingly and resubmit the patch.
Removed usage of gBrowser.userTypedValue, replaced with EventUtils.synthesizeKey("VK_BACK_SPACE", {}). Used gPrefService to set and clear "browser.urlbar.clickSelectsAll" to get consistent cross platform results (note: tested on mac os x 10.5.7)
Attachment #379572 - Attachment is obsolete: true
Comment on attachment 379591 [details] [diff] [review]
Updated test case

Perfect, thanks!
Attachment #379591 - Flags: review+
Assignee: nobody → leftturnsolutions
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/1ae4da00326e
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
Flags: in-testsuite+
Attachment #379591 - Flags: approval1.9.1?
Depends on: 494905
Verified fixed on trunk with builds on all platforms like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre ID:20090527031500
Status: RESOLVED → VERIFIED
Attachment #379591 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 379591 [details] [diff] [review]
Updated test case

a191=beltzner
Landed the second patch on branch, so that we don't trigger bug 494905 there.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b4d92ed2ebb8
Keywords: fixed1.9.1
(In reply to comment #18)
> Landed the second patch on branch, so that we don't trigger bug 494905 there.
> 
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b4d92ed2ebb8

I've done the same on trunk to resolve the intermittent failures: https://hg.mozilla.org/mozilla-central/rev/bfb383af1903
Verified fixed on 1.9.1 on all platforms with builds like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090528 Shiretoko/3.5pre ID:20090528031233
Depends on: 509295
See Also: → 1265208
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: