1673199 - Error: Permission denied to access property "mediaText" when refreshing salvatore.js.org with devtools enabled.

Status: VERIFIED FIXED

(DevTools :: Style Editor, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

see bug 1423300 comment 12.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I can reproduce that issue, and mozregression gave me https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0ae2362c2288c83b8cf0c41670288911f4ada69e&tochange=5ce60ce82c5268b5f6a219c61f13c183939a608d so it seems like a devtools issue.

Daisuke, can you take a look?

Comment 2

[Daisuke Akatsuka (:daisuke)]

Thank you very much for letting me know, Emilio.
Yes, I will take a look at this issue.

Comment 3

[Daisuke Akatsuka (:daisuke)]

Attachment: mediaText.html

Let me report as far as I investigated this issue.

It seems that if DevTools touches a field such as mediaText of MediaList earlier than the content document, this will happen.

We can confirm this problem with the attachment. The attached html define the media query at first, then access that 1sec later from js.
STRs:

1. Open html of the attachment.
2. Open DevTools and select inspector.
3. Select console.
4. Reload the content document.

If we encounter this issue, it seems that media object in the rule have been as Restricted object.
(So, permission denied?)

But, if stop referring to mediaText from following lines of DevTools , there will be no errors.
https://searchfox.org/mozilla-central/rev/a147181ece866c1ecd176ac49f112785f960aac0/devtools/server/actors/resources/stylesheets.js#379,388

That's it.
However, this may be related to management of MediaList in platform side, do you have any ideas? Emilio?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, so my guess is that we create a privileged wrapper for the MediaList object, and the page can't access it anymore. But I wonder why that is an issue for media but not for other accessors like .cssRules and such...

From a quick look, I think it's because MediaList doesn't keep any reference to the parent global (GetParentObject returns null), so when we create a reflector we associate it with the caller's global which in this case is privileged (and thus content can't access).

Olli can probably sanity-check me. We could arguably try to keep a reference to the stylesheet we came from, which would fix this I suspect.

That being said, why does the front end access .media at all? It's already accessing the .conditionText which is the same as the .media.mediaText.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1673199 - Make MediaList objects return a style sheet parent so that we associate reflectors to the right global. r=smaug

Otherwise if devtools or other privileged JS access them then content
can choke.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

The patch I attached should fix it. Can you confirm it fixes it on your end too?

Also, do you know how to best write a test for this?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f936bdfd0ee6
Make MediaList objects return a style sheet parent so that we associate reflectors to the right global. r=smaug

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1659589

Comment 9

[Cristina Coroiu [:ccoroiu]]

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

Comment 10

[Daisuke Akatsuka (:daisuke)]

It works well! Thank you very much, Emilio!
And yes, I tried to write the test, I'd like you to review it.

Comment 11

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1673199: Add a test whether it can access a filed in MediaList with normal privilege after accessing with chrome privilege.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16914662147b
Add a test whether it can access a filed in MediaList with normal privilege after accessing with chrome privilege. r=emilio

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/16914662147b

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9184479 [details]
Bug 1673199 - Make MediaList objects return a style sheet parent so that we associate reflectors to the right global. r=smaug

Beta/Release Uplift Approval Request

• User impact if declined: Some pages may misfunction with devtools enabled.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): very low-risk patch, returning a sensible global to create the object from.
• String changes made/needed: none

Comment 16

[Alin Ilea, Desktop QA]

Reproduced the initial issue in Beta 83.0b7 using Windows 10, following the steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1423300#c12
Verified - Fixed in latest Nightly 84.0a1 (build id: 20201102095541) using Windows 10 and Ubuntu 18.04. There is no Uncaught Error encountered in console after reloading the page.

Comment 17

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9184479 [details]
Bug 1673199 - Make MediaList objects return a style sheet parent so that we associate reflectors to the right global. r=smaug

Low risk patch, verified by QA on Nightly and we want web developers to have a normal browsing experience to work on Firefox Web compatibility, approved for 83 beta 8, thanks.

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e6c52aa87aac
https://hg.mozilla.org/releases/mozilla-beta/rev/4a0a8a36217a

Comment 19

[Alin Ilea, Desktop QA]

Verified - Fixed in latest Beta 83.0b8 (build id: 20201103183834) using Windows 10 and Ubuntu 18.04. There is no Uncaught Error encountered in console after reloading the page.

Comment 20

[Michael]

Thank you all!
I can verify that's fixed (Firefox 83.0, build-id: 20201112153044, agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0).
No bug in devtools on https://salvattore.js.org/ and on a new website I'm working on.