Closed Bug 967256 Opened 7 years ago Closed 3 years ago

The bookmark button should let you bookmark the page in the viewport irrespective of what's in the URL bar

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: ehsan, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P4])

STR:

1. Open a new tab.
2. Load a URL that you have previously bookmarked.  The bookmark icon glows.
3. Focus the location bar and cut the URL.  The bookmark icon loses its glow (first bug)
4. Try clicking on the bookmark button, nothing happens (second bug)
5. Press Cmd+Z to undo the cut, that won't help either.
Whiteboard: [Australis:P2]
Just tested this on win7 x64 latest tinderbox m-c win32 build.

Following the STR I see the same thing. 

However, I see another strange twist.  After step 5 if I switch tabs and then come back to the tab that was used for testing, I see a very faint 'yellow-star'.  You cannot edit or change that bookmark however.

Opening the Library and just deleting that bookmark, closing the tab, the reopening that tab with ctrl+Shit+T the Star is now 'dimmed' and its impossible to re-bookmark that particular site that was used for testing. 

Have not tried re-starting browser yet.

So, is this potential data-loss ?
restarting the browser the site I was testing with still has a Dim-star, and cannot be re-bookmarked, or edited.
Had to delete the entry from History in order to restore the bookmarking function.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #0)
> STR:
> 
> 1. Open a new tab.
> 2. Load a URL that you have previously bookmarked.  The bookmark icon glows.
> 3. Focus the location bar and cut the URL.  The bookmark icon loses its glow
> (first bug)

Actually, it's always worked like that. I'm on beta, and if I edit the URL of a bookmarked page, the star disappears entirely, so steps 4 and 5 don't even work.

> 4. Try clicking on the bookmark button, nothing happens (second bug)
> 5. Press Cmd+Z to undo the cut, that won't help either.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #1)
> Just tested this on win7 x64 latest tinderbox m-c win32 build.
> 
> Following the STR I see the same thing. 
> 
> However, I see another strange twist.  After step 5 if I switch tabs and
> then come back to the tab that was used for testing, I see a very faint
> 'yellow-star'.  You cannot edit or change that bookmark however.
> 
> Opening the Library and just deleting that bookmark, closing the tab, the
> reopening that tab with ctrl+Shit+T the Star is now 'dimmed' and its
> impossible to re-bookmark that particular site that was used for testing. 


If you just reload the page, the button works fine again. Session store (which handles ctrl-shift-t) also keeps the currently typed URL, and because you could have something completely different in the URL bar (www.foo.org as compared to www.bar.org) it's also really not clear what the desired behaviour here is.

Hiding the star entirely made a lot of sense because it was in the url bar. Now it isn't, so hiding it isn't great, either. Not sure what we want to do here. Madhava?

In the meantime, I don't think this is a regression after all, and I don't think it merits fixing for Australis v1, so I'm putting this down as a P4. Perhaps UX disagrees.
Flags: needinfo?(madhava)
Whiteboard: [Australis:P2] → [Australis:P4]
we are disabling the star when the proxystate is invalid, exactly cause the url shown may be totally different from the one bookmarked... though now that the button is not in the urlbar, we discussed making it related not to the url but to the tab contents, and if that's fine, we may stop checking proxystate and stop disabling the button in such cases.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
The bookmark button will restore vitality if focus to urlbar and press Esc key.
302 Philipp.
Flags: needinfo?(madhava) → needinfo?(philipp)
(In reply to :Gijs Kruitbosch from comment #8)
> 302 Philipp.

I'm not sure I understand what you mean by 302, but I can answer the question from earlier ;)

Since the star and the URL bar aren't connected anymore, the star should refer to whatever is loaded in the viewport.
Flags: needinfo?(philipp)
I think this will need broader places changes, so I'm moving it there.
Component: Toolbars and Customization → Bookmarks & History
Summary: The bookmark button stops to work when you cut the URL(!) → The bookmark button should let you bookmark the page in the viewport irrespective of what's in the URL bar
(In reply to :Gijs Kruitbosch from comment #10)
> I think this will need broader places changes, so I'm moving it there.

shouldn't be too complex, it's mostly the fact we were in the locationbar, and thus we were listening to the page proxy state.
I think there's also some code disallowing bookmarks for about:blank or such.
We should likely remove all those limitations and simplify the code.
Considered the Star is again bound to the Address Bar, this bug is back to the start. I'm going to resolve it because now we just hide the star when editing, rather than disabling it
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.