339725 - xlink:href don't show html link features in context menu

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[poelzi]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.3) Gecko/20060326 Firefox/1.5.0.3 (Debian-1.5.dfsg+1.5.0.3-2)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.3) Gecko/20060326 Firefox/1.5.0.3 (Debian-1.5.dfsg+1.5.0.3-2)

xlink:href links don't support features from html links like:

Open in New Tab
Copy Link Location

Nor can a middle click open a new Tab when configured.

Reproducible: Always

Comment 1

[Laxmi Harikumar]

Attachment: non-SVG testcase (generic XML with XLink)

I tried reproducing the problem with the attached testcase. But I was able to do both "Open in New Tab" and "Copy Link Location".

Build Identifier:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

Poezli, Am I understanding the problem correctly here? 

- Laxmi

Comment 2

[poelzi]

You are right. Middle click works in trunk. However, the context menu doesn't show this option, so i change the description.

Comment 3

[Philip Withnall (unavailable)]

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9a1) Gecko/20060531 Minefield/3.0a1 - Build ID: 0000000000
WFM.

Poelzi, can you reproduce this with an official Mozilla build of Firefox?

Comment 4

[poelzi]

It seems that this bug only appearce in svg files. I set the url to a svg i'm generating where every node has a link to its info page. I can't open a new tab through the submenu nor middle click.

Comment 5

[tinymammoth]

(In reply to comment #4)
> It seems that this bug only appearce in svg files. I set the url to a svg i'm
> generating where every node has a link to its info page. I can't open a new tab
> through the submenu nor middle click.
> 

I don't think it's only svg files. Go to http://www.mozilla.org/university/hof.xml. Middle click on any of the links (ie Epiphany). For me, at least, no new tab appears. 

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/2006101023 Firefox/2.0

Comment 6

[tinymammoth]

(In reply to comment #5)
> (In reply to comment #4)
> > It seems that this bug only appearce in svg files. I set the url to a svg i'm
> > generating where every node has a link to its info page. I can't open a new tab
> > through the submenu nor middle click.
> > 
> 
> I don't think it's only svg files. Go to
> http://www.mozilla.org/university/hof.xml. Middle click on any of the links (ie
> Epiphany). For me, at least, no new tab appears. 
> 
> Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1)
> Gecko/2006101023 Firefox/2.0
> 

I guess I should mention context menu items are all normal. Middle click fails to do anything, though.

Comment 7

[Aleksei Perepelkin [:chaos]]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007110515 Minefield/3.0a9pre

Testcase pass (links opened) but on other examples (http://www.mozilla.org/university/hof.xml) links won't open on middleclick, it switches to autoscroll mode and on second middleclick on same link within that mode it opens new tab. Copy link location works (not tested it on svg)

Comment 8

[jonathan chetwynd]

wfm seems to be okay in os x using latest nightlies
please check

#7 Alexey, please file a different bug

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: SVG testcase

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: SVG testcase

Comment 11

[Jonathan Watt [:jwatt]]

Attachment: SVG testcase with xlink:type="simple"

Comment 12

[jonathan chetwynd]

#9,10,11
jwatt, why are you morphing this bug?
I filed bug 407566 for the svg case.
the original test for this one was wfm
cheers

Comment 13

[Jonathan Watt [:jwatt]]

The bug is filed in the SVG component with an SVG linked in the URL field.

Comment 14

[jonathan chetwynd]

#13
jwatt the uri https://freifunk-leipzig.quamquam.org/uptime/svg/ doesn't let me in... forbidden

the original attachment clearly says non-svg, so my assumption was that this isn't an svg bug.

I'm happy to leave to your judgement, but something (the original attachment) about this bug was fixed as wfm 

Comment 16

[Takeshi Kurosawa :takenspc]

Attachment: Patch for Firefox

Comment 17

[Dimitrios Apostolou]

This 6 year old bug affects me too. I created an XVG with some <path> encapsulated inside <a xlink:href="...">, but middle click or right click don't behave properly.

How can I help fixing this?

Comment 18

[Dimitrios Apostolou]

s/XVG/SVG/

Comment 19

[Robert Longson [:longsonr]]

Were you thinking of downloading the source, fixing it and contributing a patch? Or something else?

Comment 20

[Dimitrios Apostolou]

(In reply to Robert Longson from comment #19)
> Were you thinking of downloading the source, fixing it and contributing a
> patch? Or something else?

I was thinking of submitting test cases if needed, test possible fixes on the nightlies and whatever you might think for someone who doesn't have a week to delve into the internals of mozilla codebase. 

I'm curious about the patch I can see from Kurosawa Takeshi, was it rejected or is it just waiting for review? Robert, I presume you are with Mozilla so maybe you know?

Comment 21

[Robert Longson [:longsonr]]

(In reply to Dimitrios Apostolou from comment #20)
> I was thinking of submitting test cases if needed, test possible fixes on the > nightlies and whatever you might think for someone who doesn't have a week to > delve into the internals of mozilla codebase. 

It shouldn't take a week to resurrect the existing patch, assuming that was on the right lines.

> 
> I'm curious about the patch I can see from Kurosawa Takeshi, was it rejected
> or is it just waiting for review? Robert, I presume you are with Mozilla so
> maybe you know?

I'm not with Mozilla. Looks from the history like review was never requested so it never went anywhere maybe it has issues?

Comment 22

[:Gavin Sharp [email: [e-mail]]]

I consider you to be pretty "with Mozilla", Robert :)

Comment 24

[brunoais]

Was this bug still not confirmed?

Comment 25

[Robert Longson [:longsonr]]

Attachment: unbitrotted

Comment 26

[Robert Longson [:longsonr]]

Attachment: other functions need to be consistent according to their documentation

Comment 27

[Kris Maglione [:kmag]]

Comment on attachment 8865191 [details] [diff] [review]
other functions need to be consistent according to their documentation

Sorry, I'm not the best person to review this, and my backlog is quite long at the moment, so it would be a while before I could get to it. Gijs is probably a better bet.

Comment 28

[:Gijs (he/him)]

Attachment: Bug 339725 - SVG <a> with xlink:href don't show HTML link features in context menu,

Review commit: https://reviewboard.mozilla.org/r/139406/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139406/

Comment 29

[:Gijs (he/him)]

Comment on attachment 8865191 [details] [diff] [review]
other functions need to be consistent according to their documentation

Review of attachment 8865191 [details] [diff] [review]:
-----------------------------------------------------------------

This isn't good enough, because nsContextMenu.js needs to be told how to get the URL. The .href property for SVG elements is an object with a baseVal and animVal, and we should presumably use the animVal. Plus, this needs tests.

Writing those tests and fixing that code was pretty easy, so I've just gone ahead and done it. I'll ping Jared for another review of that to be sure I'm not missing something.

Comment 30

[:Gijs (he/him)]

Comment on attachment 8867865 [details]
Bug 339725 - SVG <a> with xlink:href don't show HTML link features in context menu,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139406/diff/1-2/

Comment 31

[:Gijs (he/him)]

In fact, I think the content.js and browser.js changes are wrong too, because these are SVG links and treating them like HTML links produces the wrong result. The fallback code lower down is already designed to deal with SVG and MathML cases. Updated to remove those.

Comment 32

[Robert Longson [:longsonr]]

Comment on attachment 8867865 [details]
Bug 339725 - SVG <a> with xlink:href don't show HTML link features in context menu,

https://reviewboard.mozilla.org/r/139406/#review142750

r+ assuming the review comments are addressed.

::: browser/base/content/test/general/subtst_contextmenu.html:73
(Diff revision 2)
>  <input id="test-select-input-text-type-password" type="password" value="password">
>  <embed id="test-plugin" style="width: 200px; height: 200px;" type="application/x-test"></embed>
>  <img id="test-longdesc" src="ctxmenu-image.png" longdesc="http://www.mozilla.org"></embed>
>  <iframe id="test-srcdoc" width="98"  height="98" srcdoc="Hello World" style="border: 1px solid black"></iframe>
> +<svg id="svg-with-link"><a xlink:href="http://example.com/" xlink:type="simple"><circle cx="50%" cy="50%" r="50%" fill="blue"/></a></svg>
> +<svg id="svg-with-link2"><a xlink:href="http://example.com/" xlink:type="simple"><circle cx="50%" cy="50%" r="50%" fill="blue"/></a></svg>

was one of these supposed to be href and the other xlink:href?

I've never actually seen any content other than this set xlink:type so perhaps that's best omitted.

Comment 33

[:Gijs (he/him)]

(In reply to Robert Longson from comment #32)
> > +<svg id="svg-with-link"><a xlink:href="http://example.com/" xlink:type="simple"><circle cx="50%" cy="50%" r="50%" fill="blue"/></a></svg>
> > +<svg id="svg-with-link2"><a xlink:href="http://example.com/" xlink:type="simple"><circle cx="50%" cy="50%" r="50%" fill="blue"/></a></svg>
> 
> was one of these supposed to be href and the other xlink:href?
> 
> I've never actually seen any content other than this set xlink:type so
> perhaps that's best omitted.

No, one of these was supposed to not have the xlink:type="simple". Will fix for check-in assuming that continues to pass locally.

Comment 34

[Robert Longson [:longsonr]]

In that case you really should have 4 tests, the other two would use href rather than xlink:href

Comment 35

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8867865 [details]
Bug 339725 - SVG <a> with xlink:href don't show HTML link features in context menu,

https://reviewboard.mozilla.org/r/139406/#review142760

We support SVG with href without having to use xlink. Do we need to make any changes to support plain 'href' attribute in the context menu too?

Comment 36

[Robert Longson [:longsonr]]

(In reply to Robert Longson from comment #34)
> In that case you really should have 4 tests, the other two would use href
> rather than xlink:href

actually 3 tests would do. I can't see anyone writing href and xlink:type together.

Comment 37

[:Gijs (he/him)]

Comment on attachment 8867865 [details]
Bug 339725 - SVG <a> with xlink:href don't show HTML link features in context menu,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139406/diff/2-3/

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd7f4e79194b
SVG <a> with xlink:href don't show HTML link features in context menu, r=jaws,longsonr+218550

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dd7f4e79194b