1127588 - Frequent "Unable to run script because scripts are blocked internally" error in GMail

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Vladan Djeric (:vladan)]

I get this error in my browser console whenever I navigate between messages in my GMail inbox by using the left/right message navigation errors in the top-right part of the GMail page.

https://mail.google.com/mail/u/0/#inbox/14b2d7b3692ecaa0 : Unable to run script because scripts are blocked internally.

This is on single-process Firefox (not E10S).

Coincidentally, GMail sometimes refuses to load (gets stuck on loading screen with progress-bar partially filled) in a pinned GMail tab in my Nightly 38. Reloading doesn't help. Only switching to GMail basic HTML mode and back to full mode fixes it.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I also have the problem with gmail sometimes not loading. I'm not sure I see any relation here though. The IPC code only blocks scripts in the child process, which you're not using. Scripts are also blocked for many other reasons, which can result in the warning. I don't see it myself, so it might be caused by an add-on.

I agree it's worth investigating why gmail fails to load sometimes.

Comment 2

[Rob Wu [:robwu]]

I tried to investigate Gmail's slowness (=frozen Firefox for a couple of seconds whenever I switch views, e.g. from inbox to message) and had nothing to go on except for this error message.
I guess that I have found the cause of the error message, but it is not obvious whether it is also a cause for the slowness.


The minimal reproduction case is as follows (tested in Firefox 51 and Nightly (53), e10s and non-e10s):
1. Visit any web page with a <title> element in the document, e.g. https://example.com
2. Open the console.
3. Register the DOMSubTreeModified or DOMNodeRemoved event. The event target does not need to be inserted to the document, e.g. this works too:
   document.createElement('div').addEventListener('DOMSubtreeModified', function() {});
4. Now assign a value to the document's title (this mutates the text node in the <title> element of the document):
   document.title = document.title
   // or document.title = Math.random(); // Value does not matter. Anything works.

Result:
The global error console shows the following error:
https://example.com/ : Unable to run script because scripts are blocked internally.


I THINK that the following happens behind the scenes:
1. nsDocument::SetTitle [1] is called when document.title is assigned to.
2. mozAutoDocUpdate is constructed on the stack by nsDocument::SetTitle, which adds a script blocker [2].
3. nsContentUtils::SetNodeTextContent is called by nsDocument::SetTitle on the existing <title> element in the document [3].
4. nsContentUtils::SetNodeTextContent mutates the DOM (remove text node, insert text node in <title>) [4].
5. nsContentUtils::MaybeFireNodeRemoved is called, and invokes WarnScriptWasIgnored to write the above error message to the console [5].


[1] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsDocument.cpp#6687
[2] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/mozAutoDocUpdate.h#21-35
[3] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsDocument.cpp#6741
[4] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsContentUtils.cpp#4780-4811
[5] http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/base/nsContentUtils.cpp#4311-4337

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Thanks, Rob! This seems like a DOM bug. I doubt it explains the slowness you're experiencing (unless you're getting a ton of warnings). But we should fix it anyway.

Comment 4

[Andrew Overholt [:overholt]]

We should see if we can get this on someone's list at some point, Hsin-Yi.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Andrew Overholt [:overholt] from comment #4)
> We should see if we can get this on someone's list at some point, Hsin-Yi.

Let me work with the team to put it into the queue.

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5)
> (In reply to Andrew Overholt [:overholt] from comment #4)
> > We should see if we can get this on someone's list at some point, Hsin-Yi.
> 
> Let me work with the team to put it into the queue.

Stone is going to investigate this. Thanks Stone.

Comment 7

[GµårÐïåñ]

Has there been anything on this, because I am on the 56 branch and it is still happening, I just experienced it on my own site just now and the last update was 11 months ago that someone is going to look into it, anything come of it?

Comment 8

[John R]

I too am still seeing this bug on Firefox ESR 52.7.3, although I was not able to reproduce in Firefox 57.0.2.  Will a fix be provided for ESR?

Comment 9

[Wayne Mery (:wsmwk)]

>  Will a fix be provided for ESR?

No bug numbers are mentioned here so I would guess you won't see this resolved in ESR 52.  ESR 60 is around the corner.  Now might be a good time to test beta 60.

Comment 10

[Andrew Overholt [:overholt]]

Might be an interesting bug to look at, Edgar.

Comment 11

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

I could reproduce this on recent m-c. I will take a look.

Comment 12

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: test.html

And this issue causes changing title won't fire the 'DOMNodeRemoved' mutation event.

Comment 13

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=25c0b257ef02b575e26687e777d3ee97542f513b&filter-tier=1&group_state=expanded&selectedJob=189732552

Comment 14

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

We intend to fire the DOMNodeRemoved mutation events before a script blocker, but for issue case, we already add a script blocker in [1]. However, the mozAutoDocUpdate in [1] could not be removed since we need it for the case that inserting a new title element is needed. I guess we could change the flow here which makes mozAutoDocUpdate only be created for no title element case (which will not need to file 'DOMNodeRemoved' mutation event).

[1] https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/dom/base/nsDocument.cpp#6240

Comment 15

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bb97a80b5f0af0875b77e710a6a55c072257e59

Comment 16

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1127588 - Fix the DOMNodeRemoved mutation event isn't fired when updating document.title;

Comment 17

[Phabricator Automation]

Comment on attachment 8997833 [details]
Bug 1127588 - Fix the DOMNodeRemoved mutation event isn't fired when updating document.title;

Olli Pettay [:smaug] has approved the revision.

https://phabricator.services.mozilla.com/D2774

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e01e57d4633f
Fix the DOMNodeRemoved mutation event isn't fired when updating document.title; r=smaug

Comment 19

[Arthur Iakab [arthur_iakab]]

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