Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

making uuid field on OpaqueOrigin public #159

Closed
wants to merge 1 commit into from

Conversation

@Chandler
Copy link

Chandler commented Jan 18, 2016

by default, fields on tuple structs are private[1]. In this case, an instance of OpaqueOrigin is not very useful if the only field is private and there's no getter.

[1] https://github.com/rust-lang/rfcs/blob/master/text/0001-private-fields.md

Review on Reviewable

@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 18, 2016

The point is that you can only compare these for equality and change them. Exposing the internal representation is explicitly not something we want to allow.

@Ms2ger Ms2ger closed this Jan 18, 2016
@Chandler
Copy link
Author

Chandler commented Jan 18, 2016

@Ms2ger hmm, I was confused, now I'm more confused. If this value is private, what will print out in servo when you call location.origin in the case where a normal (protocol, host, port) origin couldn't be created?

I assumed it would be the value of the uuid in the OpaqueOrigin but it seems that's not totally obvious.
whatwg/url#21
jsdom/whatwg-url#10

I'm working on adding origin to servo's location and url APIs. I guess I can just have it return "protocol://" like in chrome?

Chrome:

new URL("accc://abcsdd.com/adf").origin
"accc://"

@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 18, 2016

https://html.spec.whatwg.org/multipage/browsers.html#unicode-serialisation-of-an-origin

If the origin in question is not a scheme/host/port tuple, then return the literal string "null" and abort these steps.

@KiChjang
Copy link
Member

KiChjang commented Jan 18, 2016

@jdm has an open PR that works on the origin issue: servo/servo#8658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.