Bug 84216 – REGRESSION(r112177): Numbered list item rendered bulleted

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[David Barr]

What steps will reproduce the problem?
1. Create a <OL style="list-style-type: decimal;">
2. Append <LI> children with "list-style-position: outside;"

What is the expected result?
A numbered list item

What happens instead?
A bulleted list item

Confirmed with WebKit nightly, Version 5.1.5 (6534.55.3, r114368).

[David Barr]

Ran bisect-builds:
Works: r112169  Fails: r112225
http://trac.webkit.org/log/trunk/?rev=112225&stop_rev=112170

[David Barr]

Consistent with my first guess, manually narrowed to:
http://trac.webkit.org/changeset/112177

"cssText should use shorthand notations"
https://bugs.webkit.org/show_bug.cgi?id=81737

[David Barr]

Also related:
http://trac.webkit.org/changeset/112321

"cssText should not generate literal 'initial' in shorthand properties"
https://bugs.webkit.org/show_bug.cgi?id=82364

Patching WebCore::StylePropertySet::getShorthandValue to return inherit or initial respectively:
list-style: inherit outside inherit;

Whereas, without that change the inspector displayed:
list-style: outside;
list-style-type: initial;
list-style-image: initial;

Could be a shorthand parsing issue.

[Alexey Proskuryakov]

<rdar://problem/11270257>

[Ryosuke Niwa]

Apparently my change exposed an existing bug in parser :(

[David Barr]

I've thought about it a bit more after discussing on IRC.
The cause appears to be that StylePropertySet::getShorthandValue()
is not bailing out when there are not enough longhand rules defined
to form a corresponding shortHand rule.

[David Barr]

Created attachment 137793 [details]
Patch

Uploading without new tests to determine test impact.

[WebKit Review Bot]

Attachment 137793 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Barr]

Downstream bug report: http://crbug.com/122606

[David Barr]

Created attachment 137811 [details]
Patch

There is at least one shorthand that does not imply ommitted values, -webkit-transform-origin.

[David Barr]

Created attachment 137814 [details]
Patch

Updated css3/flexbox/css-properties.html

[WebKit Review Bot]

Comment on attachment 137814 [details]
Patch

Attachment 137814 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12437083

New failing tests:
fast/css/transform-inline-style.html
fast/css/uri-token-parsing.html
fast/css/transform-inline-style-remove.html

[WebKit Review Bot]

Created attachment 137823 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[David Barr]

Created attachment 137827 [details]
Patch

Updated LayoutTests/fast/css/uri-token-parsing.html again. Expectations are embedded in the test, yuck.

[WebKit Review Bot]

Comment on attachment 137827 [details]
Patch

Attachment 137827 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12439113

New failing tests:
fast/css/transform-inline-style.html
fast/css/transform-inline-style-remove.html

[WebKit Review Bot]

Created attachment 137839 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[David Barr]

Created attachment 137854 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 137854 [details]
Patch

r=me provided a comment is added as to why no test is added per IRC discussion.

[David Barr]

Created attachment 138007 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 138007 [details]
Patch for landing

Clearing flags on attachment: 138007

Committed r114699: <http://trac.webkit.org/changeset/114699>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.