Closed
Bug 1336891
Opened 8 years ago
Closed 7 years ago
stylo: getComputedStyle returns empty for custom property
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla56
| Tracking | Status | |
|---|---|---|
| firefox56 | --- | fixed |
People
(Reporter: xidorn, Assigned: ferjm)
References
Details
Attachments
(2 files)
Given the following code:
<!DOCTYPE html>
<p style="--foo:bar">hello</p>
<script>
var p = document.querySelector('p');
alert(getComputedStyle(p).getPropertyValue('--foo'));
</script>
It should show "bar", but stylo always shows empty string.
Probably the stylo glue code doesn't pass the computed value of custom properties to gecko.
Test test_variable_serialization_computed.html is for this.
|
||
Comment 1•8 years ago
|
||
Xidorn, are you going to take these kinds of bugs?
Flags: needinfo?(xidorn+moz)
|
||
Updated•8 years ago
|
Component: DOM: CSS Object Model → CSS Parsing and Computation
Priority: -- → P2
|
|
||
Comment 3•8 years ago
|
||
[16:24:27] <xidorn> there is only one remaining issue as far as I know that, getComputedStyle doesn't return value for custom properties
[16:24:49] <xidorn> which is probably because we don't generate nsStyleVariables from ServoComputedValues
|
||
Comment 4•7 years ago
|
||
More specifically, Servo_GetStyleVariables returns an empty style variable structure.
Assignee: nobody → josh
|
|
||
Updated•7 years ago
|
Assignee: josh → nobody
|
Assignee | |
Updated•7 years ago
|
Assignee: nobody → ferjmoreno
Blocks: 1375230
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 7•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8880345 [details]
Bug 1336891 - stylo: Implement custom property value getter.
https://reviewboard.mozilla.org/r/151710/#review156684
r=me, with those. Thanks!
We should get a bug on file for CalcStyleDifference, which is the other user of StyleVariables(). Please ensure there's one on file for it.
::: layout/style/nsComputedDOMStyle.cpp:6920
(Diff revision 1)
> -
> nsString variableValue;
> const nsAString& name = Substring(aPropertyName,
> CSS_CUSTOM_NAME_PREFIX_LENGTH);
> - if (!variables->mVariables.Get(name, variableValue)) {
> + bool present = mStyleContext->IsServo() ?
> + Servo_GetCustomProperty(mStyleContext->ComputedValues(),
nit: I think prevailing Gecko style would be something like:
```
const bool present = mStyleContext->IsServo()
? Servo_GetCustomProperty(..)
: StyleVariables()->mVariables.Get(name, variableValue);
```
(But you can run it through clang-format if you want to confirm).
::: servo/ports/geckolib/glue.rs:3079
(Diff revision 1)
> }
> +
> +#[no_mangle]
> +pub extern "C" fn Servo_GetCustomProperty(computed_values: ServoComputedValuesBorrowed,
> + name: *const nsAString, value: *mut nsAString) -> bool {
> + match ComputedValues::as_arc(&computed_values).custom_properties() {
Let's reduce nesting from here:
```
let custom_props = match ComputedValues::as_arc(&computed_values).custom_properties() {
Some(p) => p,
None => return false,
};
let name = ..;
let value = match custom_props.get(&name) {
Some(v) => v,
None => return false,
};
value.to_css(unsafe { value.as_mut().unwrap() }).unwrap();
true
```
Also, the way you're creating an atom is really inefficient, you should be able to do:
```
Atom::from((*name).as_slice())
```
Which avoids re-encoding it as utf-8, copying into a string, and then converting that string back to utf-16, and then copying it again.
Attachment #8880345 -
Flags: review?(emilio+bugs) → review+
|
|
||
Comment 8•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8880346 [details]
Bug 1336891 - stylo: update test expectations.
https://reviewboard.mozilla.org/r/151712/#review156688
Attachment #8880346 -
Flags: review?(emilio+bugs) → review+
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 11•7 years ago
|
||
(In reply to Emilio Cobos Álvarez [:emilio] from comment #7)
> We should get a bug on file for CalcStyleDifference, which is the other user
> of StyleVariables(). Please ensure there's one on file for it.
Bug 1375536
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 14•7 years ago
|
||
Pushed by ferjmoreno@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/05a2d6ac631c
stylo: Implement custom property value getter. r=emilio
https://hg.mozilla.org/integration/autoland/rev/71a1dace62f9
stylo: update test expectations. r=emilio
|
||
Comment 15•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/05a2d6ac631c
https://hg.mozilla.org/mozilla-central/rev/71a1dace62f9
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•