Bug 159053 - CSSComputedStyleDeclaration::length should recalculate styles if needed to provide the correct value
Summary: CSSComputedStyleDeclaration::length should recalculate styles if needed to pr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-22 21:46 PDT by Joseph Pecoraro
Modified: 2016-06-23 11:50 PDT (History)
8 users (show)

See Also:


Attachments
[TEST] Reduction Test Page (455 bytes, text/html)
2016-06-22 21:46 PDT, Joseph Pecoraro
no flags Details
[PATCH] Proposed Fix (3.42 KB, patch)
2016-06-22 22:03 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (3.41 KB, patch)
2016-06-22 22:05 PDT, Joseph Pecoraro
simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
[PATCH] For Landing (3.80 KB, patch)
2016-06-23 11:14 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-06-22 21:46:44 PDT
Created attachment 281896 [details]
[TEST] Reduction Test Page

Summary:
CSSComputedStyleDeclaration::length should recalculate styles if needed to provide the correct value.

JavaScript access to the ComputedStyleDeclaration should always have the most accurate data. That should include accessing the length. Right now the length is out of date until a style recalculation or forced layout (such as accessing an item / propertyValue in the computed style).

Test:
<body>
<script>
var computedStyle = window.getComputedStyle(document.body);
var defaultLength = computedStyle.length;
document.documentElement.style.setProperty('--default-color', '#c80');
var newLength = computedStyle.length;
document.body.offsetTop;
var finalLength = computedStyle.length;

console.log("default", defaultLength);
console.log("new - with computed property", newLength);
console.log("final - after forced layout", finalLength);
</script>

Results:
Expect "new" and "final" to be equal, and both be 1 greater than "default".

Safari: (bad)
default – 331
new - with computed property – 331
final - after forced layout – 332

Firefox: (expected)
default 263
new - with computed property 264
final - after forced layout 264

Chrome:
Doesn't appear to include computed properties in Computed Style at all.
Comment 1 Joseph Pecoraro 2016-06-22 21:47:03 PDT
<rdar://problem/26638119>
Comment 2 Joseph Pecoraro 2016-06-22 22:03:45 PDT
Created attachment 281897 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2016-06-22 22:04:44 PDT
Comment on attachment 281897 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=281897&action=review

> LayoutTests/fast/css/variables/custom-property-computed-style-length-update-expected.html:2
> +<head>

Err, I should clean up the test...
Comment 4 Joseph Pecoraro 2016-06-22 22:05:28 PDT
Created attachment 281898 [details]
[PATCH] Proposed Fix
Comment 5 Simon Fraser (smfr) 2016-06-22 22:53:12 PDT
Comment on attachment 281898 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=281898&action=review

> LayoutTests/fast/css/variables/custom-property-computed-style-length-update.html:12
> +document.write(success ? "PASS" : "FAIL");

This should be a dumpAsText(). There's no point using a ref test (two snapshots and hashing) for this.
Comment 6 Joseph Pecoraro 2016-06-23 11:14:57 PDT
Created attachment 281918 [details]
[PATCH] For Landing
Comment 7 WebKit Commit Bot 2016-06-23 11:43:56 PDT
Comment on attachment 281918 [details]
[PATCH] For Landing

Clearing flags on attachment: 281918

Committed r202382: <http://trac.webkit.org/changeset/202382>