422698 - different levels of URL decoding for address bar and autocomplete suggestions

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Simon Bünzli]

Steps to reproduce:
1. Do a Bugzilla quick search for @test

Actual result:
The address bar reads
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%40test
in the autocomplete dropdown you'll see
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=@test
but selecting that entry once again results in
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%40test

Expected result:
Address bar and autocomplete list should display the same address. Currently, hitting [Down] doesn't always produce a list with at least the current address, which makes deleting individual entries from history more of a venture than should be.

Comment 1

[Simon Bünzli]

Bonus note: History marks
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%40test
as visited, while hovering over the link once again produces
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=@test

Do we have a strategy as for encoding?

Comment 2

[Ed Lee :Mardak]

If you search for "@test", the list shows "@test" in order to highlight the match. But when you actually select it, the url is %40 because otherwise it's not the same uri.

Comment 3

[Simon Bünzli]

(In reply to comment #2)
> the url is %40 because otherwise it's not the same uri.

The same as what? It looks like we're having quite a leaky abstraction here, which may affect mostly edge cases but could lead to significant amounts of torn out hair nonetheless.

What's wrong with displaying the exact same level of encoding in both address bar and autocomplete dropdown?

BTW: Why would you enter "@test" in the location bar if that string never shows up in any URL? OTOH I've already seen myself enter "%40test" (without success), because that string's been there several times...

Comment 4

[Simon Bünzli]

Another use case: I've currently two bug list URLs which look very similar. To verify that they're not exact the same (which could be a bug), I usually start deleting from the end until both URLs are suggested with the difference starting where the highlighting ends. Unfortunately I now don't get any autocomplete suggestions at all...

Comment 5

[Ed Lee :Mardak]

If the user searches for their email address "[e-mail]", we match and highlight it in both the title and url even though the url is actually blah%40site.com. However, when you select that item from the list, it shows the real uri which isn't "@".

You're probably running into bug 391691 where you can have both (1) http://site/email@blah and (2) http://site/email%40blah in your history.

If you submit a form that (automatically) escapes these, you should only be getting #2 in your history. However, if you manually edit the url say.. you find http://site/email%40blah and edit it to http://site/friend@other, the url gets stored with @ instead of %40.

Comment 6

[Ed Lee :Mardak]

FYI: Bug 419656 was recently fixed which makes it so the urls selected from the list remain as %40. Before that, if you selected a result with @, it would keep it as @ even though it was originally %40. So it would cause the @ uri to get added. So unless you're editing the uris, it should be more difficult to run into bug 391691.

Comment 7

[Simon Bünzli]

(In reply to comment #5)
> If the user searches for their email address "[e-mail]", we match and
> highlight it in both the title and url even though the url is actually
> blah%40site.com. However, when you select that item from the list, it shows the
> real uri which isn't "@".

Yeah, that's what comment #0 describes. IMO this behavior is wrong, though, since it breaks the use cases from comment #0, comment #1, comment #3 and comment #4 (all of which you haven't responded to, so far).

> You're probably running into bug 391691

No, the addresses were in fact different, I just had to copy them into Notepad to actually see that (whereas up to Firefox 2 and in all other browsers I wouldn't have had to do so)!

> If you submit a form that (automatically) escapes these, you should only be
> getting #2 in your history.

And yet you insist on not suggesting #2 (but only #1 instead)!

Comment 8

[Ed Lee :Mardak]

Well, I couldn't get the STR to work, but I do see a different bug.. but it might be the same thing you're describing if the bug had a more appropriate title along the lines of..

Current page that contains escaped characters don't show up when pressing down in autocomplete.

Tricky part is that the url bar does unescape some characters (non-ascii??) and not others like @.. sometimes. So the input to the autocomplete is the escaped ascii plus unescaped non-ascii when you modify the current url.

Comment 9

[Ed Lee :Mardak]

Simon: Can you try this build?

https://build.mozilla.org/tryserver-builds/2008-03-19_11:[e-mail]/

If I go to this link with that build, I can focus the location bar and hit down to see the current page.

https://bugzilla.mozilla.org/request.cgi?requester=edilee%40gmail.com&requestee=gavin.sharp%40gmail.com

Note: On trunk, when you visit that first tryserver link, you can hit down and see the page. That works with that build as well.

Comment 10

[Simon Bünzli]

(In reply to comment #9)
It works better, though IMO still somewhat inconsistently:
* quick searching for @test, focusing the address bar and hitting [Down] doesn't produce a result
* however modifying the URL, reverting and trying again succeeds
* searching the address bar for %40test fails, unless I've entered @test before
* furthermore entering %40test only produces a result on hitting [Down] while entering @test automatically causes the suggestions to pop up
* finally: you still (visually) suggest URLs which have never been entered/visited/bookmarked before... ;-)

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

This will not block the final release of Firefox 3. Any patch will need unit tests in order to be approved.

Comment 12

[Ed Lee :Mardak]

Attachment: v1

Fix and testcase! :)

Comment 13

[Ed Lee :Mardak]

Comment on attachment 311611 [details] [diff] [review]
v1

I'll land just the testcase when it's okay to do so.

Comment 14

[Ed Lee :Mardak]

Attachment: testcase v1

Comment 15

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/index.cgi/rev/b6d7d87ded46

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b1pre) Gecko/20080913115846 Minefield/3.1b1pre

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080913020424 Minefield/3.1b1pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1pre) Gecko/20080913031911 Minefield/3.1b1pre

I tested and verified that:

[1.] "%40test" yields the original bugzilla URL, with:
A. https://bugzilla.mozilla.org/buglist.cgi?query_format=specific&order=relevance+desc&bug_status=__open__&content=@test as the selected/suggested autocomplete choice, but:
B. https://bugzilla.mozilla.org/buglist.cgi?query_format=specific&order=relevance+desc&bug_status=__open__&content=%40test as the actual URL
[2.] "@test" does both 1A and 1B, above