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

Style computed value of a URL is returning the relative URL #17625

Closed
asajeffrey opened this issue Jul 6, 2017 · 1 comment
Closed

Style computed value of a URL is returning the relative URL #17625

asajeffrey opened this issue Jul 6, 2017 · 1 comment
Labels

Comments

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 6, 2017

If you call style.computed_value_to_string(some-url-valued-property) then you get the original, possibly relative, URL rather than the computed absolute URL. The culprit is

let string = match self.original {
Some(ref original) => &**original,
None => match self.resolved {
Some(ref url) => url.as_str(),
// This can only happen if the url wasn't specified by the
// user *and* it's an invalid url that has been transformed
// back to specified value via the "uncompute" functionality.
None => "about:invalid",
}
, which prioritizes the original URL, together with the fact that we share code between specified and computed URLs.

@asajeffrey
Copy link
Member Author

@asajeffrey asajeffrey commented Jul 6, 2017

@fnune fnune mentioned this issue Jul 20, 2017
4 of 5 tasks complete
bors-servo added a commit that referenced this issue Jul 20, 2017
…-original, r=<try>

Prioritize CSS resolved url > original url

<!-- Please describe your changes on the following line: -->

Prioritize the resolved URL instead of the original (potentially relative) URL in style.
Should probably write tests for this, but apparently this is a temporal change and Servo needs to introduce `ComputedUrl`?

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17812)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jul 24, 2017
…-original, r=<try>

Introduce ComputedUrl

<!-- Please describe your changes on the following line: -->

Use the new `ComputedUrl` type for computed types and `SpecifiedUrl` for specified types instead of using the `SpecifiedUrl` implementation for both.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because this is an implementation change and tests already exist.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17812)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jul 24, 2017
…-original, r=<try>

Introduce ComputedUrl

<!-- Please describe your changes on the following line: -->

Use the new `ComputedUrl` type for computed types and `SpecifiedUrl` for specified types instead of using the `SpecifiedUrl` implementation for both.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because this is an implementation change and tests already exist.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17812)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jul 29, 2017
…-original, r=<try>

Introduce ComputedUrl

<!-- Please describe your changes on the following line: -->

Use the new `ComputedUrl` type for computed types and `SpecifiedUrl` for specified types instead of using the `SpecifiedUrl` implementation for both.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because this is an implementation change and tests already exist.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17812)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Aug 10, 2017
…-original, r=emilio

Introduce ComputedUrl

<!-- Please describe your changes on the following line: -->

Use the new `ComputedUrl` type for computed types and `SpecifiedUrl` for specified types instead of using the `SpecifiedUrl` implementation for both.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17625 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because this is an implementation change and tests already exist.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17812)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

1 participant
You can’t perform that action at this time.