Closed
Bug 927917
Opened 11 years ago
Closed 11 years ago
Middle-clicking on the items in the History subview should open them in a new tab
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: jaws, Assigned: jaws)
References
Details
(Whiteboard: [Australis:P3][Australis:M9])
Attachments
(1 file)
2.19 KB,
patch
|
mikedeboer
:
review+
|
Details | Diff | Splinter Review |
Middling-clicking on the items in the History subview should open them in a new tab
Assignee | ||
Updated•11 years ago
|
Summary: Middling-clicking on the items in the History subview should open them in a new tab → Middle-clicking on the items in the History subview should open them in a new tab
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #818517 -
Flags: review?(mdeboer)
Assignee | ||
Comment 2•11 years ago
|
||
We don't need to do anything for Recently Closed Tabs or Recently Closed Windows since they have their own opening semantics (tabs reopen as new tabs, and windows reopen as a new window).
Comment 3•11 years ago
|
||
Comment on attachment 818517 [details] [diff] [review] Patch Review of attachment 818517 [details] [diff] [review]: ----------------------------------------------------------------- I checked if 'click' and 'command' are not both fired when a history item is clicked. They are not, so this is ready to roll. Thanks for fixing this! ::: browser/components/customizableui/src/CustomizableWidgets.jsm @@ +82,5 @@ > let item = doc.createElementNS(kNSXUL, "toolbarbutton"); > item.setAttribute("label", title || uri); > item.setAttribute("tabindex", "0"); > + item.addEventListener("command", function (aEvent) { > + onHistoryVisit(uri, aEvent, item); what I find fascinating (and not related to this bug/ patch) is that we use `item` inside a closure. Is it because `item` is block-scoped that it doesn't always reference to the last set `item` when the loop terminated? It's just a pattern that intrigues me as I've been trained to call the alarm when spotted. > I just checked this in the Web Console and the theory above is true. Awesome, long live block scoping!
Attachment #818517 -
Flags: review?(mdeboer) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Thanks for the quick review! https://hg.mozilla.org/projects/ux/rev/777283896d61
Whiteboard: [Australis:P3][Australis:M?] → [Australis:P3][Australis:M9][fixed-in-ux]
Assignee | ||
Comment 5•11 years ago
|
||
This patch also needs to be merged to m-c.
Updated•11 years ago
|
Flags: needinfo?(jaws)
Assignee | ||
Comment 6•11 years ago
|
||
Nevermind, I shouldn't look at BZ when I'm sleep deprived. This patch only touched CustomizableWidgets.jsm.
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(jaws)
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/777283896d61
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][Australis:M9][fixed-in-ux] → [Australis:P3][Australis:M9]
Target Milestone: --- → Firefox 28
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•