Right- or middle-clicking on the star icon in the location bar should not bookmark the current page

RESOLVED FIXED in Firefox 27

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cbutcher, Assigned: cbutcher)

Tracking

({regression})

Trunk
Firefox 27
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bugday-20131009])

Attachments

(2 attachments, 1 obsolete attachment)

User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.66 Safari/537.36

Steps to reproduce:

Reproducible: Always
Right- or middle-click on the star icon in the location bar.


Actual results:

A bookmark is created and the star icon becomes highlighted.


Expected results:

A bookmark should not be created, nor should the star icon become highlighted when right- or middle-clicked.
Note that right-clicking on the Reload button next to it does not trigger a reload.
Posted patch Proposed patch (obsolete) — Splinter Review
wfm: 2013-04-24-03-09-17-mozilla-central-firefox-23.0a1.en-US.linux-x86_64
australis: 2013-04-25-03-08-45-mozilla-central-firefox-23.0a1.en-US.linux-x86_64
…
australis: 2013-05-11-03-11-23-mozilla-central-firefox-23.0a1.en-US.linux-x86_64
bug: 2013-05-12-03-09-08-mozilla-central-firefox-23.0a1.en-US.linux-x86_64
Status: UNCONFIRMED → NEW
Component: Untriaged → Bookmarks & History
Ever confirmed: true
Keywords: regression
(I don't know if it was introduced at the end or in the middle/beginning)

You need to request a review of your patch from some specific developer to get it in.
Blocks: 867343
Attachment #814686 - Flags: review?(mak77)
Whiteboard: [bugday-20131009]
Assignee: nobody → cbutcher
Status: NEW → ASSIGNED
Attachment #814686 - Flags: checkin?(mak77)
Comment on attachment 814686 [details] [diff] [review]
Proposed patch

Review of attachment 814686 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing the checkin flag until this has been reviewed.
Attachment #814686 - Flags: checkin?(mak77)
Comment on attachment 814686 [details] [diff] [review]
Proposed patch

Review of attachment 814686 [details] [diff] [review]:
-----------------------------------------------------------------

it's a valid bug, though I think you should fix the caller, the onclick handler here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#557 to be something like "if (event.button === 0) BookmarkingUI.onCommand(event);"
This should not happen in UX though, even if looks like a broken merge may have reintroduced the star-icon on the urlbar?
Attachment #814686 - Flags: review?(mak77)
(In reply to Marco Bonardo [:mak] from comment #6)
> This should not happen in UX though, even if looks like a broken merge may
> have reintroduced the star-icon on the urlbar?

If you are talking about the mentions of "australis" in my comment 3, that means "the star button is a big one outside of the urlbar, and this bug doesn't happen with it".
Now checking for left-button click in caller, as per [:mak]'s suggestion.
Attachment #814686 - Attachment is obsolete: true
Attachment #817570 - Flags: review?(mak77)
Attachment #817570 - Flags: review?(mak77) → review+
Attachment #817570 - Flags: checkin?
Comment on attachment 817570 [details] [diff] [review]
star-bookmark2.patch

In the future, you can just set the checkin-needed keyword on the bug. Also, I shortened the commit message a bit to better fit with the usual standards followed.
Attachment #817570 - Flags: checkin? → checkin+
https://hg.mozilla.org/integration/fx-team/rev/fcf4875ad29e
Whiteboard: [bugday-20131009] → [bugday-20131009][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/fcf4875ad29e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [bugday-20131009][fixed-in-fx-team] → [bugday-20131009]
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.