857356 - Remove XBL field Xray auto-waiving

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (On Leave Through ~December)]

Blake thinks that anyone exposing XBL to content should just not be using fields, and that we should remove the compat shim to force them to do that. I'm ok with that, but want to wait until XBL scopes are out the door.

Comment 1

[Bobby Holley (On Leave Through ~December)]

Note to self - we should simultaneously remove the crazy waiving hack in nsXBLProtoImplField.cpp.

Comment 2

[Bobby Holley (On Leave Through ~December)]

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

Comment 3

[Bobby Holley (On Leave Through ~December)]

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

Comment 4

[Bobby Holley (On Leave Through ~December)]

Attachment: Part 1 - Remove XBL field auto-waiving. v1

Comment 5

[Bobby Holley (On Leave Through ~December)]

Attachment: Part 2 - Fix in-content XBL tests. v1

Comment 6

[Bobby Holley (On Leave Through ~December)]

Attachment: Part 3 - Convert field-y XBL tests to run with dom.use_xbl_scopes_for_remote_xul=false. v1

There are a couple of tests here that do funny things with fields. Our basic
position here is that fields have no place for in-content XBL bindings, but
there's still value in testing this stuff given our heavy usage of XBL in
chrome code. They really should be converted to chrome tests, but I was having
trouble doing that, so I decided to convert them to run without XBL scopes, like
we do for remote XUL.

As a nice side effect, this gives us a tiny bit more test coverage for the
remote XUL configuration.

Comment 7

[Bobby Holley (On Leave Through ~December)]

Attachment: Part 4 - Remove asserting crashtest. v1

This crashtest fails because it's running in the remote XUL configuration, in
which we don't have SOWs. In that case, it's no longer interesting to make
the browser assert. ;-)

Comment 8

[Bobby Holley (On Leave Through ~December)]

Attachment: Part 5 - Make reftests/crashtest run without XBL scopes. v1

I talked about this with bz. The issue is that we have a lot of XUL reftests
that end up getting run as remote XUL given that the reftest harness loads
them with file:// URIs. But realistically most of them probably want to test
the fully-featured XBL environment that we provide to frontend and extensions.

So the compromise here is to do XBL scopes for content, and no XBL scopes
for reftests/crashtests.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 747226 [details] [diff] [review]
Part 1 - Remove XBL field auto-waiving. v1

r=me

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 747227 [details] [diff] [review]
Part 2 - Fix in-content XBL tests. v1

This presumably needs to be folded into the C++ patch, right?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 747228 [details] [diff] [review]
Part 3 - Convert field-y XBL tests to run with dom.use_xbl_scopes_for_remote_xul=false. v1

>--- /dev/null
>+++ b/content/xbl/test/test_bug372769.html

Why not hg cp and preserve history?  Likewise for the other test...

r=me with that

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 747229 [details] [diff] [review]
Part 4 - Remove asserting crashtest. v1

r=me

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 747230 [details] [diff] [review]
Part 5 - Make reftests/crashtest run without XBL scopes. v1

r=me

Comment 14

[Bobby Holley (On Leave Through ~December)]

Comment on attachment 747230 [details] [diff] [review]
Part 5 - Make reftests/crashtest run without XBL scopes. v1

I think this can land. I just want jgriffin's feedback at some point to make sure all the automation prefs are set up right.

Comment 15

[Bobby Holley (On Leave Through ~December)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/52bd3c5ba8fd
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d8d4867f71c1
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a8fe89875091
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/80b26df1c79a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/02c1a0223bc3

Comment 16

[Jonathan Griffin (:jgriffin)]

Comment on attachment 747230 [details] [diff] [review]
Part 5 - Make reftests/crashtest run without XBL scopes. v1

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

Looks good to me, thanks!

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/52bd3c5ba8fd
https://hg.mozilla.org/mozilla-central/rev/d8d4867f71c1
https://hg.mozilla.org/mozilla-central/rev/a8fe89875091
https://hg.mozilla.org/mozilla-central/rev/80b26df1c79a
https://hg.mozilla.org/mozilla-central/rev/02c1a0223bc3