Bug 94108 - [css3-text] Add getComputedStyle tests for -webkit-text-decoration-line
Summary: [css3-text] Add getComputedStyle tests for -webkit-text-decoration-line
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Enhancement
Assignee: Bruno Abinader (history only)
URL:
Keywords:
Depends on: 90959 93863 94093
Blocks: 58491
  Show dependency treegraph
 
Reported: 2012-08-15 07:18 PDT by Bruno Abinader (history only)
Modified: 2013-06-03 06:41 PDT (History)
10 users (show)

See Also:


Attachments
Patch (5.75 KB, patch)
2012-08-15 07:21 PDT, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff
Patch (EWS run only) (15.46 KB, patch)
2012-08-17 06:51 PDT, Bruno Abinader (history only)
gustavo: commit-queue-
Details | Formatted Diff | Diff
Patch (14.71 KB, patch)
2012-08-20 06:31 PDT, Bruno Abinader (history only)
kenneth: review+
Details | Formatted Diff | Diff
Patch (14.68 KB, patch)
2012-08-20 06:50 PDT, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff
Patch (13.76 KB, patch)
2012-08-20 12:14 PDT, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Abinader (history only) 2012-08-15 07:18:07 PDT
Changeset r125205 introduced new CSS3 property -webkit-text-decoration-line, with included getComputedStyle* layout tests for parsing check. These were incomplete and without proper compiler flag, so bug 93863 is going to remove these additions of tests inside fast/css/getComputedStyle and svg/css/getComputedStyle.

This bug intends to create new, properly implemented getComputedStyle-based layout tests for -webkit-text-decoration-line.
Comment 1 Bruno Abinader (history only) 2012-08-15 07:21:22 PDT
Created attachment 158565 [details]
Patch

Proposed patch.
Comment 2 Bruno Abinader (history only) 2012-08-17 06:51:21 PDT
Created attachment 159111 [details]
Patch (EWS run only)

This patch adds changes to make CSS3_TEXT_DECORATION feature flag enabled and remove skip of fast/css3-text-decoration layout test directory from chromium build. This is intentional to get EWS results, but not intended for landing (as suggested by Peter Beverloo in http://lists.webkit.org/pipermail/webkit-dev/2012-August/021925.html ). It is not intended for landing, but acts as a workaround to get proper EWS results for the previous patch.
Comment 3 Gustavo Noronha (kov) 2012-08-17 07:00:34 PDT
Comment on attachment 159111 [details]
Patch (EWS run only)

Attachment 159111 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13516803
Comment 4 Bruno Abinader (history only) 2012-08-17 07:35:10 PDT
(In reply to comment #3)
> (From update of attachment 159111 [details])
> Attachment 159111 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/13516803

It appears that some platform builds doesn't handle fall-through switch cases inside #ifdef's well :/ Both builds are failing because of this:

...
    case CSSPropertyTextDecoration:
#if ENABLE(CSS3_TEXT_DECORATION)
    case CSSPropertyWebkitTextDecorationLine:
#endif // CSS3_TEXT_DECORATION
        return renderTextDecorationFlagsToCSSValue(style->textDecoration());
...

Fortunately this is fixed on patch from bug 94093 ( https://bugs.webkit.org/attachment.cgi?id=158567&action=diff#a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp_sec3 ), so I wouldn't consider these failures for now.
Comment 5 Build Bot 2012-08-17 07:39:36 PDT
Comment on attachment 159111 [details]
Patch (EWS run only)

Attachment 159111 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13514839
Comment 6 Build Bot 2012-08-17 09:08:12 PDT
Comment on attachment 159111 [details]
Patch (EWS run only)

Attachment 159111 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13528093
Comment 7 Bruno Abinader (history only) 2012-08-17 09:35:12 PDT
Adding bug 94093 as dependency (fixes build on gtk, mac and win platforms).
Comment 8 Bruno Abinader (history only) 2012-08-20 06:31:25 PDT
Created attachment 159411 [details]
Patch

Updated layout tests missing corner cases and readibility.
Comment 9 Kenneth Rohde Christiansen 2012-08-20 06:40:58 PDT
Comment on attachment 159411 [details]
Patch

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

> LayoutTests/fast/css3-text-decoration/getComputedStyle/getComputedStyle-text-decoration-line.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

<!DOCTYPE html> according to http://www.w3.org/TR/html5/syntax.html#the-doctype
Comment 10 Bruno Abinader (history only) 2012-08-20 06:50:51 PDT
Created attachment 159414 [details]
Patch

Fixed DOCTYPE tag as pointed by Kenneth.
Comment 11 Bruno Abinader (history only) 2012-08-20 12:14:23 PDT
Created attachment 159485 [details]
Patch

Updated "Reviewed by" ChangeLog section.
Comment 12 WebKit Review Bot 2012-08-20 13:44:51 PDT
Comment on attachment 159485 [details]
Patch

Clearing flags on attachment: 159485

Committed r126060: <http://trac.webkit.org/changeset/126060>
Comment 13 Bruno Abinader (history only) 2013-06-03 06:41:46 PDT
Comment on attachment 159411 [details]
Patch

Clearing cq flags, as this patch has landed already.