If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

clicking links should obey Tab preference

RESOLVED FIXED

Status

Other Applications
QA Companion
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: tracy, Assigned: Ben Hsieh)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
tested with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6

Click any link in the QA extension. 

Tested results: 
The page is opened in a new window even though I have the preference Tabs > New pages should be open in: a new tab (default)

Expected results: The linked pages open in a new tab in the existing browser window.
(Assignee)

Comment 1

10 years ago
got code that redirects the window opens, just tracking down all the links we have... I wish there was CSS to do this.
Status: NEW → ASSIGNED
(Assignee)

Updated

10 years ago
Assignee: zach → bhsieh
Status: ASSIGNED → NEW
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

10 years ago
Created attachment 275531 [details] [diff] [review]
should make links respect pref

Defines functions which will need to be called on every new link or element with links. Unfortunately, since I'm using js to register a onclick listener, it looks like there's no way to make all links do this automatically.
(Assignee)

Updated

10 years ago
Attachment #275531 - Flags: review?(zach)

Comment 3

10 years ago
Comment on attachment 275531 [details] [diff] [review]
should make links respect pref

Great. Thanks for figuring out how to do this with openUILinkIn(). Two questions for you:

Can you include utilityOverlay.js in the top level qa.xul instead of in the xul files for each tab, or does that not work for some reason?

>+ children[i].setAttribute("onclick", "qaTools.handleLink(\"" + children[i].href + "\"); return false;");
Why not just children[i].addEventHandler('click', ...)? It should be significantly faster, and doesn't involve writing js out as a string to get re-evaluated into an event handler.

>+ link.setAttribute("onclick", "qaTools.handleLink(\"" + link.href + "\"); return false;");
Same thing here.
(Assignee)

Comment 4

10 years ago
>Can you include utilityOverlay.js in the top level qa.xul instead of in the xul
files for each tab, or does that not work for some reason?

Answer seems to be no, just based on some quick experimenting. Also, note that the files you wrote (a year ago, I realize) all have some overlapping script tags at the top, so I suspect that you ran into this issue before.


>Why not just children[i].addEventHandler('click', ...)?

I admit that I didn't even know js had that function (probably because IE and FF support different versions of it). However, I fiddled around with it this morning, and it looks like onclick = "..." has one significant advantage: you can have the onclick return false, which will suppress the goto-href (opening the page in the extension window). This doesn't seem to work with addEventHandler -- which makes sense, since you can have multiple eventHandlers with that, whereas you can only have one onclick.

Anyways, I think onclick = "" is the best solution for now, unless you have any other suggestions.
(Assignee)

Comment 5

10 years ago
Another "fix" would be to dynamically modify all the hrefs passed in to point to "#" or something -- usually discouraged in webdev because it causes a jump to the top of the page, but that won't be a problem in our case. Or maybe we could even call removeAttribute so that the link doesn't have an href. Of course, then we'd have to stash the url it points to in some other attribute, like "title" or "id" or something.

All in all, I think that extracting the href and stashing it somewhere else is an even more hacky solution than just modifying the onclick.

Comment 6

10 years ago
I hate to be a pain about this, but I'm really not a big fan of the onclick="" approach here, especially if we're going to be doing it to a lot of links. You can use addEventListener() to register an onclick event handler, and then in the event handler call Event's preventDefault() method to keep it from following the href. See the DOM Event spec on event cancellation: http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-cancelation

If we can't get it to work, we can go for good here and just do it as you've written it, but writing code out as a string to be immediately recompiled isn't great.
(Assignee)

Comment 7

10 years ago
The patch in 391403 is corrected as above.

Comment 8

10 years ago
Checked in with bug 391403.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Component: QA Community Extension → QA Community Extension
Product: Webtools → Other Applications
Version: other → unspecified
You need to log in before you can comment on or make changes to this bug.