|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
5 months ago
shane and kris were going to talk about this one
The documentation is vague on what is expected here: documentUrlPatternsOptional array of string. Lets you restrict the item to apply only to documents whose URL matches one of the given match patterns. This applies to frames as well. It does however make sense that the match is against the frameUrl, not the url of the parent. At a minimum, we need to add a test form "frame" since one did not exist.
Comment on attachment 8845191 [details] Bug 1339440 use frameUrl to match context menus on frames https://reviewboard.mozilla.org/r/118380/#review121786 Still not really sold on this behavior, but fine. ::: browser/components/extensions/ext-contextMenus.js:520 (Diff revision 1) > - let pageURI = Services.io.newURI(contextData.pageUrl); > + let pageURI = contextData.inFrame ? Services.io.newURI(contextData.frameUrl) > + : Services.io.newURI(contextData.pageUrl); Please move the ternary inside the `newURI` call, or ideally something like `contextData[inFrame ? "frameUrl" : "pageUrl"]`
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s d78792d5e9a8 -d e2d9ac61dd7d: rebasing 382228:d78792d5e9a8 "Bug 1339440 use frameUrl to match context menus on frames r=kmag" (tip) merging browser/components/extensions/test/browser/browser-common.ini merging browser/components/extensions/test/browser/head.js warning: conflicts while merging browser/components/extensions/test/browser/head.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/075395ba45fe use frameUrl to match context menus on frames r=kmag
Driving by... looking at https://hg.mozilla.org/integration/autoland/rev/075395ba45fe#l1.13, I'm pretty sure: > let pageURI = Services.io.newURI(contextData["inFrame" ? "frameUrl" : "pageUrl"]); should have been: > let pageURI = Services.io.newURI(contextData[inFrame ? "frameUrl" : "pageUrl"]); The actual code that landed should always return "frameUrl" since the string "inFrame" is always truthy. I don't see where the inFrame variable comes from though, but I'll assume it's present already since comment 4 mentions it.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/9aa2aa7a42d2 use frameUrl to match context menus on frames r=kmag