Closed
Bug 427039
Opened 17 years ago
Closed 17 years ago
New Bookmark / StarUI pops up in the wrong place if the star is not on the location bar
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3
People
(Reporter: fittysix, Assigned: fittysix)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
mconnor
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040305 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040305 Minefield/3.0pre
When you type something in to the location bar the star disappears, and the go button shows up in its place. If you try to bookmark something afterwords the starui will show at the wrong place.
Reproducible: Always
Steps to Reproduce:
1. type something in the location bar
2. go to Bookmarks > Bookmark this page
Actual Results:
The starui popup will show on the left side of the content area.
Expected Results:
For visual consistency the starui should show at the end of the location bar.
The openPopup anchor is obviously the star, which is almost always there, but we could check if the go button is visible and set the anchor to that when it is.
Comment 1•17 years ago
|
||
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040402 Minefield/3.0pre
Status: UNCONFIRMED → NEW
Component: Bookmarks → Places
Ever confirmed: true
Flags: blocking-firefox3?
QA Contact: bookmarks → places
Version: unspecified → Trunk
Assignee | ||
Comment 2•17 years ago
|
||
Pops the Page Bookmark out of the go button when the star-button is not visible and the go button is visible. Tested with go button always visible userChrome mod with no problems. The original comment and code looks like content area was intentional, but with the go button we can also get the same location.
Attachment #313745 -
Flags: ui-review?
Assignee | ||
Updated•17 years ago
|
Attachment #313745 -
Flags: ui-review? → ui-review?(beltzner)
Comment 3•17 years ago
|
||
Comment on attachment 313745 [details] [diff] [review]
Patch
When the go button is showing, the content in the location bar is "dirty", and the URL shown in is *not* the URL that will be bookmarked.
I think the better solution in this case would be to:
- undo the edit to the location bar
- this will remove the Go button and return the Star
- have the drop-down appear out of the star as normal
Attachment #313745 -
Flags: ui-review?(beltzner) → ui-review-
Assignee | ||
Comment 4•17 years ago
|
||
The URL showing part is actually why I asked for ui-review, I kind of thought the same thing. That being said I don't entirely like reverting the user input since:
1. user inputs url
2. user decides to bookmark page before leaving
3. user has to re-type url
Though, since there is no indication of URL on the starui I do think reverting it is probably the better behavior, since even in the current iteration there is no indication of the URL being bookmarked.
It would be possible to save the location bar contents, and put them back after bookmarking of course, but I think that's a bit more complex of a solution than we might want to do for this, since this is a fairly edge-case problem.
Comment 5•17 years ago
|
||
Clobbering the users input to make the Star appear seems clunky to me. Is it possible to do what Ryan originally suggested; anchor the Page Bookmarked dialog to the Go button if the star is not present?
Updated•17 years ago
|
Whiteboard: FFT
Assignee | ||
Comment 6•17 years ago
|
||
This one is pretty simple, it just reverts the URL bar before ever attempting to show the popup.
Beltzner: should I save urlbar contents and re-set after the bookmark is done?
Attachment #314231 -
Flags: ui-review?(beltzner)
Comment 7•17 years ago
|
||
I think this is the right fix. Here's why:
1) The user is entering a URL to navigate away from the page (or do another page)
2) They decide to interrupt themselves and bookmark the current page
We should have the UI reflect that interruption, not do something clunky like associate the bookmark with the go button.
I don't even think we need to revert back, I think the user has stopped doing what they were doing in 1 and chosen another task. Anything else is pure guesswork about user intent after bookmarking.
Comment 8•17 years ago
|
||
Comment on attachment 314231 [details] [diff] [review]
Revert URL Bar on bookmark Patch v1
Agree with mconnor, no need to keep the half-written URL.
Attachment #314231 -
Flags: ui-review?(beltzner) → ui-review+
Comment 9•17 years ago
|
||
Comment on attachment 314231 [details] [diff] [review]
Revert URL Bar on bookmark Patch v1
r=mconnor
this always reverts the URL when the star is clicked, but that's ok.
as a note, when you click the site button (Larry) we revert there as well.
Attachment #314231 -
Flags: review+
Updated•17 years ago
|
Assignee: nobody → fittysix
Flags: blocking-firefox3? → blocking-firefox3+
Keywords: checkin-needed
Whiteboard: FFT → [has patch][has reviews] FFT
Comment 10•17 years ago
|
||
I removed the reference to the bug in the comment, I think blame is sufficient.
Attachment #313745 -
Attachment is obsolete: true
Attachment #314231 -
Attachment is obsolete: true
Comment 11•17 years ago
|
||
I checked this in pending approval, since it seems pretty clear that we want to take this.
mozilla/browser/base/content/browser-places.js 1.126
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews] FFT
Target Milestone: --- → Firefox 3
Comment 12•17 years ago
|
||
Comment on attachment 315434 [details] [diff] [review]
unbitrotted patch
requesting post-hoc approval.
Attachment #315434 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #315434 -
Flags: approval1.9? → approval1.9+
Comment 13•17 years ago
|
||
Verified with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008041404 Minefield/3.0pre ID:2008041404
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041406 Minefield/3.0pre ID:2008041406
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Comment 14•16 years ago
|
||
This is an edge case that really goes outside the scope of our FFT's...setting the in-litmus- flag.
Flags: in-litmus? → in-litmus-
Comment 15•16 years ago
|
||
I'm not sure this is such an edge case, we blocked on this for Firefox 3...
Flags: in-litmus- → in-litmus?
Comment 16•16 years ago
|
||
Aakash, the specific placement of the dialog can be added to expected results in the existing test case.
Comment 17•16 years ago
|
||
The test case https://litmus.mozilla.org/show_test.cgi?id=5954 was updated on litmus for regression testing this bug.
Flags: in-litmus? → in-litmus+
Comment 18•15 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•