817476 - _Really_ convert Screen to WebIDL

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

We sort of failed to SetIsDOMBinding(), so it's still getting XPConnect objects and all that.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Really do WebIDL bindings for Screen.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Real try push: https://tbpl.mozilla.org/?tree=Try&rev=4b1123f00196

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 687640 [details] [diff] [review]
Really do WebIDL bindings for Screen.

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

Gah, I suck at this.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hey, I reviewed, so we can blame ourselves together.  ;)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Note, due to this bug this:

  document.writeln(Screen.prototype);
  document.writeln(screen.__proto__);
  document.writeln(screen.__proto__ == Screen.prototype);

writes out two different objects and then false.  If the order of the first two calls is reversed, you get the same object (the xpconnect proto) and then true.

So we probably want to take this on Aurora.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 687640 [details] [diff] [review]
Really do WebIDL bindings for Screen.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 793244 
User impact if declined: There can be weirdness where defining a function on
   window.Screen.prototype doesn't make it appear on window.screen.
Testing completed (on m-c, etc.): Passes attached test.
Risk to taking this patch (and alternatives if risky): Low risk, I believe.  The
   other option is to back out bug 793244 which might actually be more risky.
String or UUID changes made by this patch: None.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 687640 [details] [diff] [review]
Really do WebIDL bindings for Screen.

If a backout is riskier, then please go ahead with uplifting this to Aurora so we can get bake time there.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c9e8611b83e0