Bug 32412

Summary:

css2:order of counters in out-of flow content

Product:

WebKit

Reporter:

Petri Ojala <peojala11>

Component:

CSS

Assignee:

Carol Szabo <carol>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

adele, bdakin, carol, commit-queue, darin, tkent

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	none
Proposed patch removed references to Qt	tkent: review-
Proposed Patch addressed concerns about misleading ChangeLog entry.	none
Proposed patch, Fixed indent problems	none

Petri Ojala

Reported 2009-12-11 03:44:10 PST

Steps to reproduce ------------------ 1)Provide the below link in the browser http://www.w3.org/Style/CSS/Test/CSS2.1/current/html4/counters-order-001.htm 2)observe the test page Expected result --------------- There should be only 9 boxes present with multiples of 11. Actual Result ------------- There is an extra digit 4 which is present in the test page.

Attachments
Proposed patch (40.84 KB, patch) 2010-02-01 17:08 PST, Carol Szabo	no flags	Details Formatted Diff Diff
Proposed patch removed references to Qt (40.81 KB, patch) 2010-03-03 09:33 PST, Carol Szabo	tkent: review-	Details Formatted Diff Diff
Proposed Patch addressed concerns about misleading ChangeLog entry. (41.02 KB, patch) 2010-11-10 13:14 PST, Carol Szabo	no flags	Details Formatted Diff Diff
Proposed patch, Fixed indent problems (41.05 KB, patch) 2010-11-11 09:47 PST, Carol Szabo	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Carol Szabo

Comment 1 2010-02-01 10:49:57 PST

This bug is due to the fact that the content: none; CSS spec is not correctly applied. If replaced with content: abc, WebKit works correctly. So this is an error related to the special case when none is specified. Working on a patch to see if I can get a quick fix for this.

Carol Szabo

Comment 2 2010-02-01 14:13:32 PST

Found problem in CSSParser.cpp where the case when val->unit is CSSPrimitiveValue::CSS_IDENT is not handled. The idea is that regardless of support or lack of support for the specified ident, the rule should be recorded such that previous content is cleared, isn't it?

Carol Szabo

Comment 3 2010-02-01 17:08:00 PST

Created attachment 47883 [details] Proposed patch This patch adds "none", "normal", "open-quote", "close-quote", "open-no-quote", "close-no-quote" to CSS parsing for content values. This is important for proper CSSRule application even if some of the features are not implemeted, i.e. a more specific rule content: open-quote should cancel a less specific rule content: counter(c) even if the open-quote content is not yet implemented in WebKit (see bug 6503).

Darin Adler

Comment 4 2010-02-01 17:39:33 PST

There is nothing Qt-specific here. The title should not have Qt in it, nor should the Qt keyword be attached to this bug.

Carol Szabo

Comment 5 2010-03-03 09:33:39 PST

Created attachment 49914 [details] Proposed patch removed references to Qt Removed references to Qt from the ChangeLogs

Kent Tamura

Comment 6 2010-05-20 04:28:25 PDT

Comment on attachment 49914 [details] Proposed patch removed references to Qt WebCore/ChangeLog:13 + Added support for none, normal, open-quote, no-open-quote, etc. This sentence looks like WebKit can handle open-quote, close-quote, no-open-quote, and no-close-quote correctly. We had better to have following information in ChangeLog: - recognize identifiers - no need to add implementation for none and normal - others still don't work.

Carol Szabo

Comment 7 2010-11-10 13:14:15 PST

Created attachment 73530 [details] Proposed Patch addressed concerns about misleading ChangeLog entry.

Kent Tamura

Comment 8 2010-11-10 17:31:50 PST

Comment on attachment 73530 [details] Proposed Patch addressed concerns about misleading ChangeLog entry. View in context: https://bugs.webkit.org/attachment.cgi?id=73530&action=review Looks almost ok. > LayoutTests/fast/css/content/content-none.html:16 > + function run() { > + testWidth = window.getComputedStyle(document.getElementById("testContainer"), null).getPropertyValue("width"); > + referenceWidth=window.getComputedStyle(document.getElementById("reference"), null).getPropertyValue("width"); Indentation error > LayoutTests/fast/css/content/content-none.html:27 > + <body onload="setTimeout('run()', 0);"> Is setTimeout() necessary? > LayoutTests/fast/css/content/content-normal.html:16 > + function run() { > + testWidth = window.getComputedStyle(document.getElementById("testContainer"), null).getPropertyValue("width"); > + referenceWidth=window.getComputedStyle(document.getElementById("reference"), null).getPropertyValue("width"); Indentation error > LayoutTests/fast/css/content/content-normal.html:27 > + <body onload="setTimeout('run()', 0);"> Is setTimeout() necessary?

Carol Szabo

Comment 9 2010-11-11 08:16:09 PST

(In reply to comment #8) > (From update of attachment 73530 [details]) > > LayoutTests/fast/css/content/content-normal.html:27 > > + <body onload="setTimeout('run()', 0);"> > > Is setTimeout() necessary? Maybe not for now, but while working on several counter bugs I came accross many issues that were only evidenced while using this trick. The point is that if setTimeout is not used the final configuration is reached during the first full layout, some bugs related to updating the counter tree are not apparent unless the counter tree is already built and changes are needed to it. Quotes and these content items are related to counters and affect how counters are built and shown, therefore I went for this slightly more complicated framework that would expose more bugs then otherwise. I shall fix the indentation issues and commit the patch.

Carol Szabo

Comment 10 2010-11-11 09:47:48 PST

Created attachment 73619 [details] Proposed patch, Fixed indent problems

WebKit Commit Bot

Comment 11 2010-11-11 20:36:19 PST

Comment on attachment 73619 [details] Proposed patch, Fixed indent problems Clearing flags on attachment: 73619 Committed r71882: <http://trac.webkit.org/changeset/71882>

WebKit Commit Bot

Comment 12 2010-11-11 20:36:25 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.