Arkadiusz Michalski (Spirit) Reporter
	Description • 10 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Steps to reproduce:

In observe() method for MutationObserver DOM4 make some change:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23189

Before we have:

dictionary MutationObserverInit {
	boolean childList = false;
	boolean attributes = false;
	boolean characterData = false;
	boolean subtree = false;
	boolean attributeOldValue = false;
	boolean characterDataOldValue = false;
	sequence<DOMString> attributeFilter;
};

Now options for observe is more flexible:

dictionary MutationObserverInit {
  boolean childList = false;
  boolean attributes;
  boolean characterData;
  boolean subtree = false;
  boolean attributeOldValue;
  boolean characterDataOldValue;
  sequence<DOMString> attributeFilter;
};

So at this moment, if we set some dictionary member we don't need set other dictionary member (because its set automatically), like we see in algorithm:
http://dom.spec.whatwg.org/#dom-mutationobserver-observe

This is small change and should be easy to implement. Maybe its time to correct this simple inaccuracy:)

	Arkadiusz Michalski (Spirit) Reporter
	Comment 1 • 10 years ago

Per DOM MutationObserver.observe() should throw JavaScript TypeError, not DOMException "SyntaxError":
http://dom.spec.whatwg.org/#dom-mutationobserver-observe

At now in browsers we have:
- SyntaxError (Firefox and IE)
- TypeError (Chrome)

Looks like that in Chrome 36 we have correct behaviour (including more flexible algorithm for MutationObserver.observe() method).

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 2 • 10 years ago

Want to update Gecko?
http://mxr.mozilla.org/mozilla-central/source/dom/webidl/MutationObserver.webidl and
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMMutationObserver.cpp#451

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 3 • 10 years ago

https://tbpl.mozilla.org/?tree=Try&rev=e944cda6cf9e

	Andrea Marchesini [:baku]
	Comment 4 • 10 years ago

Comment on attachment 8508930 [details] [diff] [review]
v1

Review of attachment 8508930 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsDOMMutationObserver.cpp
@@ +471,5 @@
> +  if (!aOptions.mAttributes.WasPassed() &&
> +      (aOptions.mAttributeOldValue.WasPassed() ||
> +       aOptions.mAttributeFilter.WasPassed())) {
> +    attributes = true;
> +  }

I don't know if easier to understand, but:

bool attributes = aOptions.mAttributes.WasPassed() ?
                    aOptions.mAttributes.Value() :
                    (aOptions.mAttributeOldValue.WaPassed() || aOptions.mAttributeFilter.WasPassed());

@@ +476,5 @@
> +
> +  if (!aOptions.mCharacterData.WasPassed() &&
> +      aOptions.mCharacterDataOldValue.WasPassed()) {
> +    characterData = true;
> +  }

bool characterData = aOptions.mCharacterData.WasPassed() ?
                       aOptions.mCharacterData.Value() : aOptions.mCharacterDataOldValue.WasPassed();

@@ +487,2 @@
>      return;
>    }

if (attributeOldValue &&
    aOptions.mAttributes.WasPassed() &&
    !aOptions.mAttributes.Value()) {

@@ +496,5 @@
>    }
> +
> +  if (aOptions.mCharacterDataOldValue.WasPassed() &&
> +      aOptions.mCharacterDataOldValue.Value() &&
> +      aOptions.mCharacterData.WasPassed() &&

if (characterDataOldValue &&
    aOptions.mCharacterData.WasPassed() &&
    !aOptions.mCharacterData.Value()) {

::: content/base/test/test_mutationobservers.html
@@ +124,5 @@
>      m.observe(document, { childList: true, characterDataOldValue: true });
>    } catch (ex) {
>      e = ex;
>    }
> +  ok(!e, "Should have thrown an exception");

Shouldn't

@@ +132,5 @@
>      m.observe(document);
>    } catch (ex) {
>      e = ex;
>    }
>    ok(e, "Should have thrown an exception");  

can you remove this extra space?

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 5 • 10 years ago

> 
> I don't know if easier to understand, but:
I tried to follow the step from the spec, and would prefer to keep it that way.
The local bool variables are there only because aOptions is passes as const

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 6 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc8d30fd13e7

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 10 years ago

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

	Eric Shepherd [:sheppy]
	Comment 9 • 6 years ago

This change is mentioned on Fx 61 for devs and on the main MutationObserver page, as well as to some extent on the page for observe(). I will be finishing this work when I get home from a trip.