Closed
Bug 857356
Opened 10 years ago
Closed 10 years ago
Remove XBL field Xray auto-waiving
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(5 files)
9.98 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
3.61 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
7.88 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
1.61 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
5.79 KB,
patch
|
bzbarsky
:
review+
jgriffin
:
feedback+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Note to self - we should simultaneously remove the crazy waiving hack in nsXBLProtoImplField.cpp.
Assignee | ||
Comment 2•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=20d46cb631b4
Assignee | ||
Comment 3•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=0ecdfcf407c0
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #747226 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #747227 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•10 years ago
|
||
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.
Attachment #747228 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 7•10 years ago
|
||
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. ;-)
Attachment #747229 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 8•10 years ago
|
||
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.
Attachment #747230 -
Flags: review?(jgriffin)
Attachment #747230 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•10 years ago
|
Attachment #747230 -
Attachment description: Make reftests/crashtest run without XBL scopes. v1 → Part 5 - Make reftests/crashtest run without XBL scopes. v1
![]() |
||
Comment 9•10 years ago
|
||
Comment on attachment 747226 [details] [diff] [review] Part 1 - Remove XBL field auto-waiving. v1 r=me
Attachment #747226 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 10•10 years ago
|
||
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?
Attachment #747227 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 11•10 years ago
|
||
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
Attachment #747228 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 12•10 years ago
|
||
Comment on attachment 747229 [details] [diff] [review] Part 4 - Remove asserting crashtest. v1 r=me
Attachment #747229 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 13•10 years ago
|
||
Comment on attachment 747230 [details] [diff] [review] Part 5 - Make reftests/crashtest run without XBL scopes. v1 r=me
Attachment #747230 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 14•10 years ago
|
||
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.
Attachment #747230 -
Flags: review?(jgriffin) → feedback?(jgriffin)
Assignee | ||
Comment 15•10 years ago
|
||
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•10 years ago
|
||
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!
Attachment #747230 -
Flags: feedback?(jgriffin) → feedback+
Comment 17•10 years ago
|
||
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
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•