Closed Bug 1430507 Opened 2 years ago Closed 2 years ago

Context menu Open Link In New Window does not work on svg element

Categories

(Firefox :: Menus, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: david.newcomb, Assigned: Gijs)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20180103231032

Steps to reproduce:

Env:
 Firefox 57.0.4 64bit mac
 Darwin Davids-MacBook-Pro.local 17.3.0 Darwin Kernel Version 17.3.0: Thu Nov  9 18:09:22 PST 2017; root:xnu-4570.31.3~1/RELEASE_X86_64 x86_64

1. Go to: https://www.w3.org/TR/2016/CR-SVG2-20160915/images/linking/link01.svg
2. Context (right) click on red ellipse, see Open Link in New Tab, click.
3. Nothing happens

The same think happens if you have an svg element inside an anchor e.g. < a href="blar"><svg ...></a>


Actual results:

Nothing


Expected results:

The link should be opened in a new tab/window
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
20180114220708

On right-click:

DataCloneError: The object could not be cloned.  vapi-client.js:269
Sending message that cannot be cloned. Are you trying to send an XPCOM object?  ContextMenu.jsm:624:6


On "Open Link in New Tab" click:

uncaught exception: Load of http://www.w3.org/ denied.  (unknown)


:Gijs, you reported bug 1225052. Now the menu entries are there, so that's fixed, but they don't work.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(gijskruitbosch+bugs)
OS: Unspecified → All
Hardware: Unspecified → All
Component: Untriaged → SVG
Product: Firefox → Core
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Component: SVG → Menus
Flags: needinfo?(gijskruitbosch+bugs)
Product: Core → Firefox
Version: 57 Branch → Trunk
Duplicate of this bug: 1225052
Depends on: 1429709
Comment on attachment 8942617 [details]
Bug 1430507 - pass something that's always serializable for SVG elements and the context menu,

https://reviewboard.mozilla.org/r/212890/#review218848

Thanks for the test! Looks ok to me, just holding off on r+'ing to see what you think of checking that TEST_LINK was actually opened in the new (private?) window.

::: browser/base/content/test/contextMenu/browser_contextmenu_linkopen.js:9
(Diff revision 2)
> +"use strict";
> +
> +const TEST_LINK = "https://example.com/";
> +const RESOURCE_LINK = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "https://example.com") + "test_contextmenu_links.html";
> +
> +async function activeContextAndWaitFor(selector, which) {

Naming suggestions:
s/active/activate and s/which/where

::: browser/base/content/test/contextMenu/browser_contextmenu_linkopen.js:22
(Diff revision 2)
> +      closeMethod = async (tab) => BrowserTestUtils.removeTab(tab);
> +      break;
> +    case "privatewindow":
> +      contextMenuItem += "private";
> +      openPromise = BrowserTestUtils.waitForNewWindow().then(win => {
> +        ok(PrivateBrowsingUtils.isWindowPrivate(win), "Should have opened a private window.");

Can we also check that TEST_LINK was opened in the window? (same goes for the non-private-window case)

::: browser/base/content/test/contextMenu/test_contextmenu_links.html:10
(Diff revision 2)
> +</head>
> +<body>
> +Browser context menu link subtest.
> +
> +<a id="test-link" href="https://example.com">Click the monkey!</a>
> +<a id="test-image-link" href="/"><img src="ctxmenu-image.png"></a>

This <img> tag is unclosed.
Attachment #8942617 - Flags: review?(nhnt11)
(In reply to Nihanth Subramanya [:nhnt11] from comment #6)
> ::: browser/base/content/test/contextMenu/browser_contextmenu_linkopen.js:9
> (Diff revision 2)
> > +"use strict";
> > +
> > +const TEST_LINK = "https://example.com/";
> > +const RESOURCE_LINK = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "https://example.com") + "test_contextmenu_links.html";
> > +
> > +async function activeContextAndWaitFor(selector, which) {
> 
> Naming suggestions:
> s/active/activate and s/which/where

Fair points.

> ::: browser/base/content/test/contextMenu/browser_contextmenu_linkopen.js:22
> (Diff revision 2)
> > +      closeMethod = async (tab) => BrowserTestUtils.removeTab(tab);
> > +      break;
> > +    case "privatewindow":
> > +      contextMenuItem += "private";
> > +      openPromise = BrowserTestUtils.waitForNewWindow().then(win => {
> > +        ok(PrivateBrowsingUtils.isWindowPrivate(win), "Should have opened a private window.");
> 
> Can we also check that TEST_LINK was opened in the window? (same goes for
> the non-private-window case)

I guess that works, yeah...

> ::: browser/base/content/test/contextMenu/test_contextmenu_links.html:10
> (Diff revision 2)
> > +</head>
> > +<body>
> > +Browser context menu link subtest.
> > +
> > +<a id="test-link" href="https://example.com">Click the monkey!</a>
> > +<a id="test-image-link" href="/"><img src="ctxmenu-image.png"></a>
> 
> This <img> tag is unclosed.

It's HTML, you don't need to close <img> tags (nor <input>, <meta>, ...)
Comment on attachment 8942617 [details]
Bug 1430507 - pass something that's always serializable for SVG elements and the context menu,

https://reviewboard.mozilla.org/r/212890/#review218894

Thanks!
Attachment #8942617 - Flags: review?(nhnt11) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c2239403f21a
pass something that's always serializable for SVG elements and the context menu, r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/c2239403f21a
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.