[Webkit-unassigned] [Bug 28828] Make SVGStyleElement title/media/type setters do something

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28828


Cameron McCormack <cam at mcc.id.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38811|                            |review?
               Flag|                            |
  Attachment #38774|0                           |1
        is obsolete|                            |




--- Comment #3 from Cameron McCormack <cam at mcc.id.au>  2009-08-31 06:19:17 PDT ---
Created an attachment (id=38811)
 --> (https://bugs.webkit.org/attachment.cgi?id=38811)
Patch v2

(In reply to comment #2)
> shouldBeEqualToString will make your life easier here:
>  5 shouldBe("style.type", "'text/css'");
> 
> shouldBeEqualToString("style.type", "text/css");

OK, changed to use that.  I just copied some other test that was using
shouldBe() in this manner.

> Normally we would use longer argument names here:
>  53 void SVGStyleElement::setXmlspace(const AtomicString& s, ExceptionCode&)
> like "space"
> 
> and "type":
>  65 void SVGStyleElement::setType(const AtomicString& s)

Changed.

> Why not just remove "setter raises()"?
>  33                      /*setter raises(DOMException)*/;
> 
> Are these spec'd to raise, but they never should?  If so, please explain in
> your ChangeLog.

The spec says to raise these in case the DOM node is read only, but in reality
this is never the case.  Strictly, it can happen if the <style> is a descendant
of an Entity or EntityReference node.  Various methods would need to check
isReadOnly() and throw a NO_MODIFICATION_ALLOWED_ERR in this case.  Does
XMLTokenizerLibxml2 ever cause parsed XML documents to include EntityReference
nodes instead of expanding them?  I couldn't tell from a quick look.

> In general this looks great!  If you were a committer, I would r+ and you could
> land with changes.

Thanks!  WebKit code is pretty nice to work with. :-)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.