Closed Bug 1144015 Opened 11 years ago Closed 11 years ago

(Browser API) Fire mozbrowseropentab on ctrl-click and middle click

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox40 --- fixed

People

(Reporter: paul, Assigned: paul)

References

Details

Attachments

(1 file, 4 obsolete files)

v2.3 (r=kchen)
11.94 KB, patch
paul
: review+
Details | Diff | Splinter Review
No description provided.
Attached patch v1 (obsolete) — — Splinter Review
Assignee: nobody → paul
Status: NEW → ASSIGNED
Attachment #8578468 - Flags: review?(justin.lebar+bug)
Blocks: graphene
Attachment #8578468 - Flags: review?(justin.lebar+bug) → review?(kchen)
Comment on attachment 8578468 [details] [diff] [review] v1 Review of attachment 8578468 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/browser-element/BrowserElementChildPreload.js @@ +593,5 @@ > + // Open in a new tab if middle click or ctrl/cmd-click. > + if ((Services.appinfo.OS == 'Darwin' && e.metaKey) || > + (Services.appinfo.OS != 'Darwin' && e.ctrlKey) || > + e.button == 1) { > + sendAsyncMsg('openwindow', { Use a new event name? The original mozbrowseropenwindow has a different meaning.
Attachment #8578468 - Flags: review?(kchen) → feedback+
And test case please :)
Attached patch v2 (obsolete) — — Splinter Review
Attachment #8578468 - Attachment is obsolete: true
Attachment #8584431 - Flags: review?(kchen)
Comment on attachment 8584431 [details] [diff] [review] v2 Review of attachment 8584431 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/browser-element/mochitest/mochitest.ini @@ +122,5 @@ > # process. Default is OOP. > [test_browserElement_NoAttr.html] > [test_browserElement_NoPref.html] > [test_browserElement_NoPermission.html] > +[test_browserElement_OpenTab.html] So this test case works in the inproc variant. Does it work out-of-process? Create _inproc_ and _oop_ tests accordingly. ::: dom/browser-element/mochitest/test_browserElement_OpenTab.html @@ +1,4 @@ > +<!DOCTYPE HTML> > +<html> > +<!-- > +https://bugzilla.mozilla.org/show_bug.cgi?id=741587 Wrong link
Attachment #8584431 - Flags: review?(kchen) → feedback+
Attached patch v2.1 (obsolete) — — Splinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb9c5e20cf0a
Attachment #8584431 - Attachment is obsolete: true
Test times out on emulator. But it doesn't make sense to test this on touch-only platforms.
Attached patch v2.2 (obsolete) — — Splinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2cbd837de80
Attachment #8585438 - Attachment is obsolete: true
Summary: (Browser API) Fire mozbrowseropenwindow on ctrl-click and middle click → (Browser API) Fire mozbrowseropentab on ctrl-click and middle click
Attachment #8586560 - Flags: review?(kchen)
Comment on attachment 8586560 [details] [diff] [review] v2.2 Review of attachment 8586560 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. One last thing: put test_browserElement_inproc_OpenTab.html in mochitest.ini, test_browserElement_oop_OpenTab.html in mochitest-oop.ini
Attachment #8586560 - Flags: review?(kchen) → review+
(In reply to Kan-Ru Chen [:kanru] from comment #10) > Comment on attachment 8586560 [details] [diff] [review] > v2.2 > > Review of attachment 8586560 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good. One last thing: put test_browserElement_inproc_OpenTab.html in > mochitest.ini, test_browserElement_oop_OpenTab.html in mochitest-oop.ini inproc won't work as it's intercepted by Firefox tabs in browser mochitest.
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/65ff49ab095d Going off the assumption that those m-dt failures on the Try push were from a bad parent rev and not this patch. We'll see :)
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/65ff49ab095d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: