Rename CrossOriginWrapper

RESOLVED FIXED in mozilla17

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: krizsa)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

The name is terribly confusing. Now that NoWaiverWrapper is gone, we should fix this up.

Luke suggested TransitivelyWaivingWrapper, which sounds fine to me, unless anyone has a better suggestion.

Blake, how does TransitivelyWaivingWrapper sound?
(Assignee)

Comment 1

5 years ago
Yaaay. Also, once we have the name you can assign this one to me anytime.
How about WaiveXrayWrapper? TransitivelyWaiving sounds like it could waive just about anything.
(Assignee)

Comment 3

5 years ago
(In reply to Blake Kaplan (:mrbkap) from comment #2)
> How about WaiveXrayWrapper? 

Sounds good, but isn't that kind of taken already (WaiveXrayWrapperWrapper)?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3)
> (In reply to Blake Kaplan (:mrbkap) from comment #2)
> > How about WaiveXrayWrapper? 
> 
> Sounds good, but isn't that kind of taken already (WaiveXrayWrapperWrapper)?

We should just rename that to XrayWaiver IMO.
(Assignee)

Comment 5

5 years ago
So XrayWaiver and WaiveXrayWrapper are a bit too easy to mix up for me. One of the trivial difference between them is one of them is CCW the other is SCW. Would it make sense to make that difference explicit in their names to avoid confusion?

If I'm the only one who finds them easy to mix up, I like them already better than the current names. Btw what is against simply calling it TransparentWrapper?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #5)
> So XrayWaiver and WaiveXrayWrapper are a bit too easy to mix up for me. One
> of the trivial difference between them is one of them is CCW the other is
> SCW. Would it make sense to make that difference explicit in their names to
> avoid confusion?

They accomplish two very distinct things. The second isn't just the SCW version of the first - it's serves as a second identity to make the operation of the first possible.

In our parlance, an object "has a waiver" (a noun) if it's bound inside XrayWaiver. We then use the WaiveXrayWrapper to access it.

How about s/WaiveXrayWrapper/WaivedXrayWrapper/? I think that's more precise, and might help alleviate the confusion a little bit.
 
> If I'm the only one who finds them easy to mix up, I like them already
> better than the current names. Btw what is against simply calling it
> TransparentWrapper?

Because that's much less explicit about what's actually going on, IMO. Waiving Xray gives transparent semantics, but it _also_ clamps the principal. And that would also imply that same-origin content wrappers are somehow not transparent.
(Assignee)

Comment 7

5 years ago
(In reply to Bobby Holley (:bholley) from comment #6)
> They accomplish two very distinct things. 

I know, but it seemed like an easy way to distinguish them.

> In our parlance, an object "has a waiver" (a noun) if it's bound inside
> XrayWaiver. We then use the WaiveXrayWrapper to access it.

This sold it to me.

> How about s/WaiveXrayWrapper/WaivedXrayWrapper/? I think that's more
> precise, and might help alleviate the confusion a little bit.

I prefer the other one. Only one character difference is too easy for the eyes and the ears to mix up.
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #7)
> > In our parlance, an object "has a waiver" (a noun) if it's bound inside
> > XrayWaiver. We then use the WaiveXrayWrapper to access it.
> 
> This sold it to me.

w00t! Care to whip up a patch? ;-)
(Assignee)

Updated

5 years ago
Assignee: nobody → gkrizsanits
(Assignee)

Comment 9

5 years ago
Created attachment 642405 [details] [diff] [review]
part1: Rename CrossOriginWrapper class
Attachment #642405 - Flags: review?(bobbyholley+bmo)
(Assignee)

Comment 10

5 years ago
Created attachment 642406 [details] [diff] [review]
part2: Rename CrossOriginWrapper files
Attachment #642406 - Flags: review?(bobbyholley+bmo)
(Assignee)

Comment 11

5 years ago
Created attachment 642407 [details] [diff] [review]
part3: Rename WaiveXrayWrapperWrapper
Attachment #642407 - Flags: review?(bobbyholley+bmo)

Updated

5 years ago
Attachment #642405 - Flags: review?(bobbyholley+bmo) → review+

Updated

5 years ago
Attachment #642406 - Flags: review?(bobbyholley+bmo) → review+

Comment 12

5 years ago
Comment on attachment 642407 [details] [diff] [review]
part3: Rename WaiveXrayWrapperWrapper

Much better naming.
Attachment #642407 - Flags: review?(bobbyholley+bmo) → review+
(Assignee)

Comment 13

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=1dfd8c22d506

Comment 14

5 years ago
\o/
(Assignee)

Comment 15

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/e55d5df611b9
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4aae5703855
http://hg.mozilla.org/integration/mozilla-inbound/rev/ecc4f73417d8
Didn't make it to mozilla-central before the uplift (merge was blocked on bug 774259). Adjusting milestone accordingly.
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/e55d5df611b9
https://hg.mozilla.org/mozilla-central/rev/f4aae5703855
https://hg.mozilla.org/mozilla-central/rev/ecc4f73417d8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.