1395767 - Changing attributes in a MutationObserver hangs FF

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Ryan Jentzsch]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170817115854

Steps to reproduce:

See this JSFiddle: https://jsfiddle.net/RyanNerd/60xw1mbd/6/
Stretch the textarea element to more than 300px. 


Actual results:

FF Hangs when the style attribute is changed inside the MutationObserver callback function.

Note: I'm guessing that this is a recursion issue since attributes are being observed by the MutationObserver and within the callback function the observed attribute gets changed.

My platform info:
FF 55.0.2 (64-bit)
Linux Mint 18.1



Expected results:

FF should not hang when an attribute is updated in a MutationObserver callback function. The Fiddle works without issue in Chrome 60.

Comment 1

[Ryan Jentzsch]

Bug is present in this release (I assume it is also present in 56 and 57).

Comment 2

[Ryan Jentzsch]

Fixing status to include 56 and 57.

Comment 3

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

Unless this is a very widespread problem we are unlikely to fix this for 55.

Comment 4

[Alice0775 White]

At least on windows, the hang is reproduced since Firefox 14.
So, this seems the implementation level problem.

Comment 5

[Ryan Jentzsch]

This is not a major bug and the work-around is to issue a takeRecords()` after making changes to the observed attribute within the call-back function. This clears the event queue and prevents the recursion I believe.` 
I posted this bug so you are aware of the issue and also anything that can consistently crash or hang a process is a potential security risk in my experience.

Comment 6

[Jet Villegas (inactive)]

(In reply to Ryan Jentzsch from comment #5)
> This is not a major bug and the work-around is to issue a takeRecords()`
> after making changes to the observed attribute within the call-back
> function. This clears the event queue and prevents the recursion I believe.` 
> I posted this bug so you are aware of the issue and also anything that can
> consistently crash or hang a process is a potential security risk in my
> experience.

Thanks for the report. 

The workaround suggests that this is an issue with MutationObserver and not CSS. Moving to get the right folks looking into it.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Chrome has had issues with dealing with nested mutations, for example
https://bugs.chromium.org/p/chromium/issues/detail?id=634005

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Hmm, but why is there any attribute change when one resizes textarea.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Aha, style attribute is modified both in Chrome an Firefox.
And that is supposed to change the attribute
https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface

But I guess the question is that whether calling target.style.backgroundColor ="blue"; when the background color is already blue is supposed to trigger attribute change.

Comment 10

[Ryan Jentzsch]

(In reply to Olli Pettay [:smaug] from comment #9)
> Aha, style attribute is modified both in Chrome an Firefox.
> And that is supposed to change the attribute
> https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
> 
> But I guess the question is that whether calling
> target.style.backgroundColor ="blue"; when the background color is already
> blue is supposed to trigger attribute change.

Yes. The example fiddle is intentionally bad programming practice and a second work-around for this issue is to check if the style attribute needs to be modified: if (target.style.background.color !== "blue") {target.style.background.color="blue"};

Comment 11

[Julien Cristau [:jcristau]]

Old bug, sounds minor, setting status to fix-optional and not tracking.

Comment 12

[Ryan Jentzsch]

(In reply to Julien Cristau [:jcristau] from comment #11)
> Old bug, sounds minor, setting status to fix-optional and not tracking.

I'm going to repeat this (not that I think that this is always so or present in this case). However, in my experience any code that can consistently hang or crash a process carries with it a potential security exploit. Although an old bug it may potentially carry a security risk (once again not that I have found one in this case. Just stating from my experience).

Comment 13

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

I can't reproduce using the testcase from comment 0 as of bug 1473180 landing.

Hey Ryan, did bug 1473180 totally fix the issue here, or is there a variation of the test case that will still hang?

Comment 14

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

(In reply to Mike Conley (:mconley) (:⚙️) from comment #13)
> I can't reproduce using the testcase from comment 0 as of bug 1473180
> landing.
> 

To be clear, bug 1473180 landed in Nightly only 25 days ago, so to test variations, you'll need to use Firefox Nightly.

Comment 15

[Xidorn Quan [:xidorn] UTC+10]

This should have been fixed by the combination of the two bugs mentioned in depends on.

Comment 16

[Ryan Jentzsch]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #13)
> I can't reproduce using the testcase from comment 0 as of bug 1473180
> landing.
> 
> Hey Ryan, did bug 1473180 totally fix the issue here, or is there a
> variation of the test case that will still hang?

I'm not able to test. My version of FF is 62.0b16 (64-bit). If the fiddle I provided doesn't hang then yes the bug is fixed.

Comment 17

[Cristian Fogel [:cfogel]]

Managed to reproduce the issue on 62.0.2.
Verified with 63.0b9 and 64.0a1 (2018-09-27) on Windows 10, MacOS 10.9, Ubuntu 16.04 and did not encounter any issue after the resize (no hang/freezes).