Open
Bug 454534
Opened 16 years ago
Updated 2 years ago
Some browser commands should be disabled for about:blank
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
NEW
People
(Reporter: Natch, Unassigned)
Details
(Keywords: polish, ue, useless-UI, Whiteboard: [needs new patch])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
asaf
:
review-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
It would probably be more correct to just disable the "Bookmark This Page" in the Bookmarks menu, but this is the reality when clicking "Bookmark this Page" on a new tab the panel is incorrectly positioned.
Reproducible: Always
Steps to Reproduce:
1.open a new tab/window
2.Select Bookmarks-->Bookmark this Page
Actual Results:
Panel is off to the left.
Expected Results:
Bookmark this page in about:blank should be grayed out?
Reporter | ||
Comment 1•16 years ago
|
||
Just a note if it is disabled, just think of the many context menus that would need tweaking for this (i.e. browser context, tabs context, etc.).
Updated•16 years ago
|
Component: Bookmarks & History → Places
OS: Windows Vista → All
QA Contact: bookmarks → places
Hardware: PC → All
Version: unspecified → Trunk
Reporter | ||
Comment 3•16 years ago
|
||
The real question is, where to put it? There is no star icon in about:blank page, so what would be more appropriate? Note: I *really* don't think it makes sense to disable the option, as there are so many paths to cover, this should be handled, the question is how?
Comment 4•16 years ago
|
||
As you said, the correct fix would probably to disable bookmarking for that page, since we do that for the star we should be able to do that for the Bookmark This Page.
Alternatively if we want to allow that or that's too complex, we could use the New Bookmark dialog instead of the contextual popup when the star is not available for an uri, but the preferred way of fixing this is the first imo.
Updated•16 years ago
|
Assignee: nobody → dao
Component: Places → General
QA Contact: places → general
Summary: Bookmark panel positioned improperly (to the left) on about:blank page → Shouldn't be able to bookmark about:blank
Comment 5•16 years ago
|
||
Looks like this will be a bigger cleanup patch...
Summary: Shouldn't be able to bookmark about:blank → Some browser commands should be disabled for about:blank
Comment 6•16 years ago
|
||
Attachment #357922 -
Flags: review?(mano)
Reporter | ||
Comment 7•16 years ago
|
||
Can't you use (content.document == ImageDocument)?
Comment 8•16 years ago
|
||
We want to exclude more than just images there, like videos. The use of mimeTypeIsTextBased seems fine to me.
Reporter | ||
Comment 9•16 years ago
|
||
That's interesting, maybe that can be used for bug 462892 and bug 466909... instead of adding the interface.
Comment 10•16 years ago
|
||
We should add the interface either way, since we need to be able to detect videos reliably. Excluding text/[x]html is not sufficient for controlling whether video-related commands appear.
Comment 11•16 years ago
|
||
Attachment #357922 -
Attachment is obsolete: true
Attachment #375524 -
Flags: review?(mano)
Attachment #357922 -
Flags: review?(mano)
Reporter | ||
Comment 12•15 years ago
|
||
Would be nice if this made 1.9.2
Flags: wanted-firefox3.6?
Flags: in-testsuite?
Whiteboard: [needs review mano]
Comment 13•15 years ago
|
||
Comment on attachment 375524 [details] [diff] [review]
updated to trunk
1. The "This Frame" menu isn't handled, leading to inconsitten behavior (at least for page info).
2. I'm not sure that it's a smart idea to disable page info for about:blank.
3. I don't like the changes to the all-tabs handler. I would rather either merge it to PlacesCommandHook somehow, or leave it as is (and please do not remove the queryinterface method).
4. nit: spaces between methods
Also, In context menus we generally hide rather than disable items. Ask ux.
Attachment #375524 -
Flags: review?(mano) → review-
Updated•15 years ago
|
Whiteboard: [needs review mano]
Updated•15 years ago
|
Assignee: dao → nobody
Flags: wanted-firefox3.6?
Reporter | ||
Updated•14 years ago
|
Whiteboard: [needs new patch]
Updated•2 years ago
|
Severity: minor → S4
You need to log in
before you can comment on or make changes to this bug.
Description
•