1207629 - Telemetry broke viewing files from DOM Inspector add-on

Status: RESOLVED FIXED

(Toolkit :: View Source, defect)

Description

[:Gijs (he/him)]

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.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Sorry... :(

I'll fix it up.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: 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

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Gah sorry - I didn't realize you'd nabbed it!

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

[Tracking Requested - why for this release]: Let's be sure to fix in 43 to cover everyone.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/integration/fx-team/rev/dd9395054bf20baeb899fcdc8e9fda79389e01bf
Bug 1207629 - Don't assume that viewSourceUtils.js has Services in scope. r=jryans

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Thanks for the fast review!

Comment 8

[Carsten Book [:Tomcat]]

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

Comment 9

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d5adbfd01a52