Closed
Bug 574006
Opened 14 years ago
Closed 14 years ago
Support context menu and open-in-new-tab behavior
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mfinkle, Assigned: mfinkle)
Details
Attachments
(1 file, 3 obsolete files)
20.91 KB,
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
e10s will require context menu code to be split between chrome and content process. Also, the open-in-new-tab via CTRL+Tap needs to be supported as well. This is a WIP patch. CTRL+Tap works, but context menu support doesn't yet. The "contextmenu" event is not being fired.
Attachment #453390 -
Flags: feedback?(21)
Assignee | ||
Updated•14 years ago
|
Attachment #453390 -
Flags: feedback?(mbrubeck)
Comment 1•14 years ago
|
||
Comment on attachment 453390 [details] [diff] [review] wip 1 > ContentCustomClicker.prototype = { >- _dispatchMouseEvent: function _dispatchMouseEvent(aName, aX, aY) { >+ _dispatchMouseEvent: function _dispatchMouseEvent(aName, aX, aY, aModifiers) { > let browser = this._browserView.getBrowser(); > let [x, y] = Browser.transformClientToBrowser(aX, aY); >- browser.messageManager.sendAsyncMessage(aName, { x: x, y: y }); >+ browser.messageManager.sendAsyncMessage(aName, { x: x, y: y, modifiers: aModifiers }); > }, aModifiers || null; > > mouseDown: function mouseDown(aX, aY) { > // Ensure that the content process has gets an activate event > let browser = this._browserView.getBrowser(); > let fl = browser.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader; > try { > fl.activateRemoteFrame(); > } catch (e) {} >- this._dispatchMouseEvent("Browser:MouseDown", aX, aY); >+ this._dispatchMouseEvent("Browser:MouseDown", aX, aY, null); > }, remove null here and everywhere otherwise it looks fine, I'm not sure why the contextmenu is not working on e10s but it works on mozila-central. the merge from yesterday night seems to have took into account the change into nsEventStateManager done for handling it so i think it would be fine after rebuild
Attachment #453390 -
Flags: feedback?(21) → feedback+
Assignee | ||
Comment 2•14 years ago
|
||
Adds some fixes now that I can actually see a context menu. Adds some code from Vivien to enable on remote tabs Still has dragging problems on local pages, but remote pages seem to work ok
Assignee: nobody → mark.finkle
Attachment #453390 -
Attachment is obsolete: true
Attachment #453390 -
Flags: feedback?(mbrubeck)
Comment 3•14 years ago
|
||
(In reply to comment #2) > Created an attachment (id=454069) [details] > wip 2 > > Adds some fixes now that I can actually see a context menu. > Adds some code from Vivien to enable on remote tabs > > Still has dragging problems on local pages, but remote pages seem to work ok It seems to work for me when I remote the setTimeout around: element.ownerDocument.releaseCapture(); Also I think we need to ignore the contextmenu event if a pan has occured (MouseCancel), there is no clean way to cancel it now (or at least none that I know). I'll be ok with a flag for now while we've not found a better solution. A part of the code in input handler is also here to stop panning while the context menu is show (bug 555725)
Assignee | ||
Comment 4•14 years ago
|
||
This patch seems to have everything working, except canceling the "click" when a contextmenu has been shown.
Attachment #454069 -
Attachment is obsolete: true
Assignee | ||
Comment 5•14 years ago
|
||
* Added mouse cancel check in ContentCustomClicker.singleClick Seems to work well in e10s for local and remote pages
Attachment #454856 -
Attachment is obsolete: true
Attachment #454858 -
Flags: review?(21)
Comment 6•14 years ago
|
||
Comment on attachment 454858 [details] [diff] [review] patch - let rects = getContentClientRects(element); - sendAsyncMessage("Browser:Highlight", { rects: rects }); - }, kTapOverlayTimeout); + this._sendMouseEvent("mousedown", element, x, y); + element.ownerDocument.releaseCapture(); A comment could be nice for the releaseCapture, because this can look unclear why there is a call to releaseCapture without any setCapture
Attachment #454858 -
Flags: review?(21) → review+
Assignee | ||
Comment 7•14 years ago
|
||
added comment pushed: http://hg.mozilla.org/mobile-browser/rev/50239a7fa3c3
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•