The default bug view has changed. See this FAQ.

contextMenus.OnClickData should have the frameId (can be used with tabs.sendMessage)

UNCONFIRMED
Unassigned

Status

()

Toolkit
WebExtensions: Frontend
P5
normal
UNCONFIRMED
a month ago
16 hours ago

People

(Reporter: kernp25, Unassigned, Mentored)

Tracking

({good-first-bug})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [contextMenus], triaged)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

a month ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170214030231

Steps to reproduce:

Chrome also has frameId in info
https://developer.chrome.com/extensions/contextMenus#event-onClicked

Updated

a month ago
Keywords: good-first-bug
Priority: -- → P5
Whiteboard: [contextMenus], triaged
any work been done on this? can I start on it?
Flags: needinfo?(kernp25)
(Reporter)

Comment 2

15 days ago
Created attachment 8847177 [details] [diff] [review]
Bug 1339416 - contextMenus.OnClickData should have the frameId (can be used with tabs.sendMessage)
Flags: needinfo?(kernp25)
Attachment #8847177 - Flags: feedback?(hrishikeshbman)
(Reporter)

Comment 3

15 days ago
(In reply to hrishikeshbman from comment #1)
> any work been done on this?
Yes! See the patch.

> can I start on it?
Yes!
(Reporter)

Comment 4

7 days ago
Created attachment 8850063 [details] [diff] [review]
bug-1339416-fix.patch
Attachment #8847177 - Attachment is obsolete: true
Attachment #8847177 - Flags: feedback?(hrishikeshbman)
Attachment #8850063 - Flags: review?(mixedpuppy)
Comment on attachment 8850063 [details] [diff] [review]
bug-1339416-fix.patch

This looks fine, I'd like to see a test along with this.  Maybe add something in browser_ext_contextMenus_onclick.js.  There is a frame now in the context.html test file that could be used as part of that.

Also, if the context menu is on chrome, what does frameId end up as?  I'm expecting it should be -1.  You could probably add a simple check for that in some test in browser_ext_contextMenus_chrome.js
Attachment #8850063 - Flags: review?(mixedpuppy) → feedback+
(Reporter)

Comment 6

6 days ago
(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> Comment on attachment 8850063 [details] [diff] [review]
> bug-1339416-fix.patch
> 
> This looks fine, I'd like to see a test along with this.  Maybe add
> something in browser_ext_contextMenus_onclick.js.  There is a frame now in
> the context.html test file that could be used as part of that.
> 
> Also, if the context menu is on chrome, what does frameId end up as?  I'm
> expecting it should be -1.  You could probably add a simple check for that
> in some test in browser_ext_contextMenus_chrome.js

How to write and run those tests?
(Reporter)

Comment 7

5 days ago
Created attachment 8851064 [details] [diff] [review]
bug-1339416-fix-v2.patch
Attachment #8850063 - Attachment is obsolete: true
Attachment #8851064 - Flags: review?(kmaglione+bmo)
Comment on attachment 8851064 [details] [diff] [review]
bug-1339416-fix-v2.patch

Still needs tests I requested
Attachment #8851064 - Flags: review?(kmaglione+bmo) → review-
(Reporter)

Comment 9

3 days ago
Created attachment 8851341 [details] [diff] [review]
bug-1339416-fix-v3.patch
Attachment #8851341 - Flags: feedback?(scaraveo)
(Reporter)

Comment 10

3 days ago
113 INFO TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_contextMenus_chrome.js | click in frame - Got 4294967304, expected 1

Can you tell me what's the problem?
(Reporter)

Updated

3 days ago
Attachment #8851341 - Flags: feedback?(scaraveo) → feedback?(mixedpuppy)
Mentor: mixedpuppy@gmail.com
You need to log in before you can comment on or make changes to this bug.