Changing attributes in a MutationObserver hangs FF

UNCONFIRMED
Unassigned

Status

()

Core
CSS Parsing and Computation
P3
normal
UNCONFIRMED
8 months ago
7 months ago

People

(Reporter: Ryan Jentzsch, Unassigned, NeedInfo)

Tracking

({regression, regressionwindow-wanted})

57 Branch
x86_64
All
regression, regressionwindow-wanted
Points:
---

Firefox Tracking Flags

(firefox55- wontfix, firefox56- fix-optional, firefox57- fix-optional)

Details

(Reporter)

Description

8 months ago
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.
(Reporter)

Comment 1

8 months ago
Bug is present in this release (I assume it is also present in 56 and 57).
status-firefox55: --- → ?
tracking-firefox55: --- → ?
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Version: 55 Branch → 57 Branch
(Reporter)

Comment 2

8 months ago
Fixing status to include 56 and 57.
status-firefox56: --- → ?
status-firefox57: --- → ?
tracking-firefox56: --- → ?
tracking-firefox57: --- → ?
Unless this is a very widespread problem we are unlikely to fix this for 55.
status-firefox55: ? → wontfix
Keywords: regression, regressionwindow-wanted

Comment 4

7 months ago
At least on windows, the hang is reproduced since Firefox 14.
So, this seems the implementation level problem.
OS: Linux → All

Updated

7 months ago
Priority: -- → P3
(Reporter)

Comment 5

7 months ago
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

7 months ago
(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.
Component: DOM: CSS Object Model → DOM: Core & HTML

Comment 7

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

Comment 8

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

Comment 9

7 months ago
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.
Component: DOM: Core & HTML → CSS Parsing and Computation
Flags: needinfo?(dbaron)
(Reporter)

Comment 10

7 months ago
(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"};
Old bug, sounds minor, setting status to fix-optional and not tracking.
status-firefox56: ? → fix-optional
status-firefox57: ? → fix-optional
tracking-firefox55: ? → -
tracking-firefox56: ? → -
tracking-firefox57: ? → -
(Reporter)

Comment 12

7 months ago
(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).
You need to log in before you can comment on or make changes to this bug.