Last Comment Bug 667195 - Ctrl + Click on SVG/MathML links does not open pages in a new tab
: Ctrl + Click on SVG/MathML links does not open pages in a new tab
: regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: Firefox 7
Assigned To: Marco Bonardo [::mak]
Depends on: 847895
Blocks: 549340
  Show dependency treegraph
Reported: 2011-06-25 08:00 PDT by Frédéric Wang (:fredw)
Modified: 2013-03-05 10:47 PST (History)
7 users (show)
mak77: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (1.12 KB, application/xhtml+xml)
2011-06-25 08:00 PDT, Frédéric Wang (:fredw)
no flags Details
patch v1.0 (9.83 KB, patch)
2011-06-28 18:25 PDT, Marco Bonardo [::mak] review+
Details | Diff | Splinter Review

Description User image Frédéric Wang (:fredw) 2011-06-25 08:00:50 PDT
Created attachment 541930 [details]

The testcase contains various kinds of links. Using Ctrl + Click on HTML links open pages in a new tab. However, this does not work for MathML/SVG links.
Comment 1 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-26 19:54:01 PDT
The code that handles this isn't part of the core; it's part of the Firefox UI.
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-26 20:00:04 PDT
In particular, see |function contentAreaClick| in browser/base/content/browser.js

Presumably hrefAndLinkNodeForClickEvent is failing here?  In particular, it doesn't seem to stop when it finds an XLink.   Looks like the changes in bug 549340 lost realHref and just completely broke this code for XLinks....
Comment 3 User image Marco Bonardo [::mak] 2011-06-27 07:07:00 PDT
looks like my regression, taking.

Yes it's possibly a bug in the while (node) loop, next iteration may overwrite href.
Comment 4 User image Marco Bonardo [::mak] 2011-06-28 18:25:47 PDT
Created attachment 542702 [details] [diff] [review]
patch v1.0

As thought, the loop was the culprit.
I added all the examples in the testcase attached to the test for contentAreaClick, and made subtests being able to run on multiple targets.
Comment 5 User image :Gavin Sharp [email:] 2011-06-28 18:39:53 PDT
Comment on attachment 542702 [details] [diff] [review]
patch v1.0

The old code found the outermost element with a valid xlink href, seems like we'd want to keep that behavior (rather than returning the innermost, as this patch does).
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-28 18:55:26 PDT
You want the innermost.  Since that code went away Gecko's link handling has changed to take the innermost link when clicking on nested links.
Comment 7 User image :Gavin Sharp [email:] 2011-06-28 19:35:54 PDT
Oh, I thought the transition went the opposite way for some reason.
Comment 8 User image Bill Gianopoulos [:WG9s] 2011-06-29 09:54:46 PDT
I can verify that the patch here, not only fixes the Ctrl-click issue for links in the testcases in this bug, but also the additional ones in bug 427990.

The 29 Jun Windows and Linux builds available at do/will include this patch.
Comment 9 User image Marco Bonardo [::mak] 2011-06-30 12:16:29 PDT
I'm going to remove the subtest for mathhref from the test because we are not yet supporting that.
To clarify, MathML 3 allows to add a href attribute to any MathML element, till now we supported XLinks on MathML but not a direct href attribute. This is not a regression, has been fixed just some days ago in central and contentAreaClick never handled it.
I have a patch for that, but being completely unrelated to this regression I'm going to file a bug apart and move the subtest there.
Comment 10 User image Marco Bonardo [::mak] 2011-06-30 13:19:56 PDT
I filed bug 668618 to add support for MathML 3 href links.
Comment 11 User image Marco Bonardo [::mak] 2011-06-30 13:41:58 PDT
Pushed with other stuff to bake, will merge as soon as I have a green run.
subtest removal:
Comment 13 User image Vlad [QA] 2011-07-07 06:38:23 PDT
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0a2) Gecko/20110706 Firefox/7.0a2

Note You need to log in before you can comment on or make changes to this bug.