Convert HTMLStyleElement to WebIDL

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
4 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

Trunk
mozilla21
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

No description provided.
Attachment #709407 - Flags: review?(bzbarsky)
Comment on attachment 709406 [details] [diff] [review]
Part 1: Rename nsHTMLStyleElement to mozilla::dom::HTMLStyleElement

Maybe put the DOMCI_NODE_DATA up top near NS_IMPL_NS_NEW_HTML_ELEMENT?  That's how we've done it elsewhere.

> +++ b/content/html/content/src/HTMLStyleElement.h

So... the way I've been doing this sort of thing is to create the .h via a hg cp from the original file so that blame is preserved on the lines you leave in it.

Please fix that; I hate losing blame.  And that would make reviewing easier too.. ;)

r=me with that fixed.
Attachment #709406 - Flags: review?(bzbarsky) → review+
Comment on attachment 709407 [details] [diff] [review]
Part 2: Move HTMLStyleElement to Web IDL bindings

>+++ b/content/base/src/nsStyleLinkElement.h
>+  nsIStyleSheet* Sheet()

LinkStyle.sheet should be nullable in the IDL.  So this should be GetSheet().  I raised http://lists.w3.org/Archives/Public/www-style/2013Feb/0105.html

We may or may not want to get rid of GetStyleSheet in favor of GetSheet().

>+++ b/content/html/content/src/HTMLStyleElement.cpp

You need to SetIsDOMBinding() in the constructor, right?

>+++ b/dom/webidl/HTMLStyleElement.webidl
>+           attribute boolean disabled;

Shouldn't that be [SetterThrows]?

r=me with those fixed.
Attachment #709407 - Flags: review?(bzbarsky) → review+
Follow-up to not throw success: hg.mozilla.org/integration/mozilla-inbound/rev/1d7232646d12

This was causing crashtest MOZ_ASSERT aborts.
Also hit some test failures, and backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/e2cb5c5978fd
Was this backed out for the M1 orange?

If so, that's unexpected passes, yes?  If so, I'd think just removing the line at http://hg.mozilla.org/mozilla-central/file/31268d71c33c/content/html/content/test/reflect.js#l65 would do the trick.
Yeah, I was just running out and didn't want to push that change without testing.  Relanded with the reflect.js hack removed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3117442985d
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff480fbd99a3
https://hg.mozilla.org/mozilla-central/rev/a3117442985d
https://hg.mozilla.org/mozilla-central/rev/ff480fbd99a3
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.