Telemetry broke viewing files from DOM Inspector add-on

RESOLVED FIXED in Firefox 43

Status

()

Toolkit
View Source
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Gijs, Assigned: mconley)

Tracking

({regression})

unspecified
mozilla44
regression
Points:
---

Firefox Tracking Flags

(firefox42 unaffected, firefox43+ fixed, firefox44+ fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
STR:

0. Install dom inspector add-on
1. open nightly
2. open dom inspector (the add-on),
3. inspect chrome document
4. inspect a node
5. go to CSS rule view on the right
6. right click any rule on the right
7. click "view file"


ER:
see file

AR:
nothing

The browser console yields:

ReferenceError: Services is not defined
gViewSourceUtils._openInInternalViewer()
 viewSourceUtils.js:162
gViewSourceUtils.viewSource()
 viewSourceUtils.js:65
ViewFileURIBase_DoTransaction()
 baseCommands.js:155
execCommand()
 inspector.xml:276
oncommand()
 styleRules.xul:1

which got broken by bug 1203624, because that assumed Services.jsm was available, which it might not be in the scopes where viewSourceUtils.js gets included. It should either avoid using Services.jsm or import it.
Sorry... :(

I'll fix it up.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
status-firefox42: --- → unaffected
status-firefox43: --- → affected
status-firefox44: --- → affected
Created attachment 8664885 [details]
MozReview Request: Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r?jryans

Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r?jryans
Attachment #8664885 - Flags: review?(jryans)
Gah sorry - I didn't realize you'd nabbed it!
Assignee: jryans → mconley
Comment on attachment 8664885 [details]
MozReview Request: Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r?jryans

https://reviewboard.mozilla.org/r/20051/#review18065

No worries, looks good.  We should uplift to 43 as well.
Attachment #8664885 - Flags: review?(jryans) → review+
[Tracking Requested - why for this release]: Let's be sure to fix in 43 to cover everyone.
tracking-firefox43: --- → ?
Thanks for the fast review!
https://hg.mozilla.org/mozilla-central/rev/dd9395054bf2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Comment on attachment 8664885 [details]
MozReview Request: Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r?jryans

Approval Request Comment
[Feature/regressing bug #]:

Bug 1203624

[User impact if declined]:

Some add-ons that use viewSourceUtils.js might not be able to work properly if Services is not available in the scope that viewSourceUtils.js runs in.

[Describe test coverage new/current, TreeHerder]:

A night of baking on fx-team, and has now merged into central. View source has a number of automated tests, which are all passing. DOM Inspector (the add-on that we noticed was broken) was also tested with this patch, and now works again.


[Risks and why]: 

Extremely low risk. The patch is trivial - it just makes available the Services singleton if it's not already available in scope.


[String/UUID change made/needed]:

None.
Attachment #8664885 - Flags: approval-mozilla-aurora?
tracking-firefox43: ? → +
tracking-firefox44: --- → +
Keywords: regression
Comment on attachment 8664885 [details]
MozReview Request: Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r?jryans

Existing tests pass, let's uplift this to aurora.
Attachment #8664885 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.