Closed Bug 419324 Opened 13 years ago Closed 3 years ago

Places needs to unescape URLs in the UI (library tree/edit, bookmark tooltip/status, etc)

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED INACTIVE

People

(Reporter: Mardak, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

When bug 415460 lands, the url listed in the treeview will correctly be unescaped, but clicking a bookmark to show more information about it has the %escaped% url because it uses uri.spec.

We can keep the two consistent by showing what the tree row shows.
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #305374 - Flags: review?(mano)
hrm, but what happens once you change the field contents?
Oh interesting. The tree view shows %bl%ah%ju%nk but if you close the window and reopen it, it shows the value from the database.

I'm assuming there was an optimization to avoid hitting the DB again when the location changed?
Comment on attachment 305374 [details] [diff] [review]
v1

(In reply to comment #2)
> hrm, but what happens once you change the field contents?
Updated the patch in bug 415460 to support incremental/live bookmark update, so the url in the tree view is also unescaped. So this patch is good as is.
I suppose technically this doesn't need to wait for bug 415460 to land. Except the tree will be showing escaped URLs while the edit panel has unescaped.
Attached patch v2 (obsolete) — Splinter Review
This doesn't require the backend changes from bug 415460 that probably won't happen. But probably won't land until the JSON patch as that will be moving PlaceUtils methods around.
Attachment #305374 - Attachment is obsolete: true
Attachment #305690 - Flags: review?(mano)
Attachment #305374 - Flags: review?(mano)
No longer depends on: 415460
Attached patch v2.1Splinter Review
This fixes the URI for UI in the bookmark toolbar tooltip, edit bookmarks in Library, statusbar for sidebar, and Library tree view.

The statusbar still shows escaped URIs when hovering over the bookmark item though...
Attachment #305690 - Attachment is obsolete: true
Attachment #305705 - Flags: review?
Attachment #305690 - Flags: review?(mano)
Summary: Edit bookmarks dialog doesn't unescape URLs → Places needs to unescape URLs in the UI (libray tree/edit, bookmark tooltip/status, etc)
Summary: Places needs to unescape URLs in the UI (libray tree/edit, bookmark tooltip/status, etc) → Places needs to unescape URLs in the UI (library tree/edit, bookmark tooltip/status, etc)
Edward: what's the status here? who's the reviewer?
I believe there was a comment from mano about potentially running into performance issues because something (tree view?) would keep asking for the URLs, so if we keep unescaping them on the fly, there could be issues.
Duplicate of this bug: 460793
Comment on attachment 305705 [details] [diff] [review]
v2.1

Foreced encode of bidi is missing.
See also browser.js
This bug may be related to (or depends on ) bug 320807.
can we take a patch with bidi fixed (we can probably inherit the bidi code from browser.js losslessdecodeURI function), skipping the treeView change?
Editing is probably the more real issue users find (bug 449994, that i'm going to mark as dependant on this)
Blocks: 449994
and probably the helper should be in PlacesUIUtils
Comment on attachment 305705 [details] [diff] [review]
v2.1

please request review from an actual peer
Attachment #305705 - Flags: review?
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
Why are spaces (%20) unescaped in urls in the urlbar?  Is there an option to show them escaped?

You want spaces unescaped in places to facilitated editing, but the urlbar, at least optionally, should show the actual url.
Assignee: edilee → nobody
Status: ASSIGNED → NEW
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.