New issue
Advanced search Search tips

Issue 668478 link

Starred by 2 users
Status: Assigned
Owner:
f...@opera.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Tidy up/compat align dimension handling in HTML presentation attributes

Project Member Reported by f...@opera.com, Nov 24 2016 
The current implementation of "dimensions" [1] (residing in HTMLElement::addHTMLLengthToStyle), is a rough scan over the input followed by passing a (possibly truncated) version of the value off to the CSS parser (via StylePropertySet::setProperty) for the final say. In part this adds (some fairly minor) complexity to the CSS parser, but it also requires even setting one up, when we have a very limited value syntax to take care of.

There appear to be some discrepancies compared to the spec text and other browsers. The main one is likely about how '*' is accepted during the syntax "scan" (check), but such a value will then be rejected by the CSS parser. (Other browsers treat '*' as just plain garbage, parsing any number before it.)

There's currently a use counter active that measures '%' and '*' in 'width' presentation attributes [2]. Given how "wide" it is, gauging the amount of "failures" (from '*') is difficult. It could make sense to narrow this counter, and possible add an additional one.

[1] https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-dimension-values
[2] https://www.chromestatus.com/metrics/feature/timeline/popularity/905
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b

commit 337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b
Author: fs <fs@opera.com>
Date: Mon Nov 28 17:37:58 2016

Rework the "rules for parsing dimension values" implementation

This CL reworks the current implementation of the "rules for parsing
dimension values" [1] (HTMLElement::addHTMLLengthToStyle) into a
separate function and moves it to HTMLDimension.{cpp,h}.
In general, behavior deviating from the specced version is kept with the
following exceptions:

 * Allow all of the "space characters" [2], rather than just U+0020.

 * Cases with multiple full stops (ex: "1.2.3") now parse the same as
   "1.2" rather than failing.

Comments are added where the implementation is known to deviate from the
spec.

This also makes it possible to avoid calling into the CSS parser for
actual parsing, which should reduce the amount of special-cases needed
there. This requires a mechanism for disallowing percentage values
though, to properly handle 'cellspacing' on <table>.

[1] https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-dimension-values
[2] https://html.spec.whatwg.org/multipage/infrastructure.html#space-character

BUG=668478

Review-Url: https://codereview.chromium.org/2528673003
Cr-Commit-Position: refs/heads/master@{#434678}

[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLDimension.cpp
[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLDimension.h
[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLDimensionTest.cpp
[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLElement.cpp
[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLElement.h
[modify] https://crrev.com/337cac12ea5b43b0d8dd2d1a9fe64a8e0dbb523b/third_party/WebKit/Source/core/html/HTMLTableElement.cpp
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05e671c1e4c641e231b9c89c14b0273591a14c3a

commit 05e671c1e4c641e231b9c89c14b0273591a14c3a
Author: timloh <timloh@chromium.org>
Date: Mon Jan 09 23:36:27 2017

Replace HTMLAttributeMode with HTMLStandardMode

HTMLAttributeMode is no longer any different to HTMLStandardMode as we
now correctly parse HTML lengths in a separate routine instead of using
the CSS parser. This patch thus removes the enum and replaces its usage
with HTMLStandardMode.

BUG=668478

Review-Url: https://codereview.chromium.org/2622473002
Cr-Commit-Position: refs/heads/master@{#442402}

[modify] https://crrev.com/05e671c1e4c641e231b9c89c14b0273591a14c3a/third_party/WebKit/Source/core/css/parser/CSSParserMode.h
[modify] https://crrev.com/05e671c1e4c641e231b9c89c14b0273591a14c3a/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp
[modify] https://crrev.com/05e671c1e4c641e231b9c89c14b0273591a14c3a/third_party/WebKit/Source/core/dom/PresentationAttributeStyle.cpp
►

Sign in to add a comment