The default bug view has changed. See this FAQ.

Style Inspector: open URLs in new tab

RESOLVED FIXED in Firefox 23

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: briangrinstead, Assigned: briangrinstead)

Tracking

Trunk
Firefox 23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Recently added the ability to open an external URL in the Style Inspector in a new window: https://bugzilla.mozilla.org/show_bug.cgi?id=677930.

We should use openUILinkIn to allow us to open multiple links in multiple tabs, as opposed to opening the resource in a new window.
(Assignee)

Comment 1

4 years ago
Created attachment 743307 [details] [diff] [review]
Initial patch that opens external URLs in new tab instead of new window
(Assignee)

Updated

4 years ago
Attachment #743307 - Flags: review?(mratcliffe)
(Assignee)

Comment 2

4 years ago
Michael,
I have submitted a patch for review.  A couple of notes:
1) I used the fat arrow function to bind `this` on the event handler.  I've seen that in use elsewhere in the devtools code, so figured this would be OK.
2) It may not be needed to actually use an <a> tag now that we are opening in a new tab, but I left it in since it would likely be just setting an attribute on a span rather than href on the a tag.
3) Regarding testing, I am currently just checking the `href` attribute to make sure it is correct - do you think it would be good to somehow also check to make sure a tab opens?

Thanks, and let me know if there is anything I can do to improve the patch.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 743307 [details] [diff] [review]
Initial patch that opens external URLs in new tab instead of new window

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

(In reply to Brian Grinstead from comment #2)
> Michael,
> I have submitted a patch for review.  A couple of notes:
> 1) I used the fat arrow function to bind `this` on the event handler.  I've
> seen that in use elsewhere in the devtools code, so figured this would be OK.

Yes, we like fat arrow functions ;o)

> 2) It may not be needed to actually use an <a> tag now that we are opening
> in a new tab, but I left it in since it would likely be just setting an
> attribute on a span rather than href on the a tag.

In this case an a tag makes sense.

> 3) Regarding testing, I am currently just checking the `href` attribute to
> make sure it is correct - do you think it would be good to somehow also
> check to make sure a tab opens?
> 

You could do that but programatically clicking things can cause problems due by focus issues. I think the current test is fine.


> Thanks, and let me know if there is anything I can do to improve the patch.

Nope, it is great as it is.
Attachment #743307 - Flags: review?(mratcliffe) → review+
Whiteboard: [land-in-fx-team]
has this run through try or through a local test run? If either answer is true, this can land?
(Assignee)

Comment 5

4 years ago
I don't have access to the try server, but I have run the local tests with mach mochitest-browser and everything is green.  I'm sure Michael could give a more definitive answer.
I will run it through try.
https://tbpl.mozilla.org/?tree=Try&rev=1333a39b38d3
got some broken builds on Win7 and Fedora.

Looking at the logs they look like infrastructure blow-outs. Retriggered the builds to see what happens.
https://tbpl.mozilla.org/?tree=Try&rev=c76ccea39da1
https://hg.mozilla.org/integration/fx-team/rev/117a4b00981a
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/117a4b00981a
Assignee: nobody → briangrinstead
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
You need to log in before you can comment on or make changes to this bug.