Bug 92659

Summary: [css] Prefer usage of RenderStyle::textDecorationsInEffect() when rendering
Product: WebKit Reporter: Bruno Abinader (history only) <bruno.abinader>
Component: CSSAssignee: Bruno Abinader (history only) <bruno.abinader>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, igor.oliveira, jchaffraix, peter, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 93863    
Bug Blocks: 58491    
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-02 none

Bruno Abinader (history only)
Reported 2012-07-30 10:45:50 PDT
WebKit currently has a proprietary CSS property '-webkit-text-decorations-in-effect', which as far as I've studied on the code and tested, is only used internally as part of the 'text-decoration' CSS property implementation (as up to CSS spec 2.1). Inside WebCore/rendering, there are mixed usage of both RenderStyle::textDecoration() and RenderStyle::textDecorationsInEffect(). To avoid a possible breach in terms of data redundancy, prefer usage of RenderStyle::textDecorationsInEffect() sounds more correct, after render style adjustments.
Attachments
Patch (5.07 KB, patch)
2012-07-30 10:50 PDT, Bruno Abinader (history only) 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from gce-cr-linux-02 (1.03 MB, application/zip)
2012-07-30 12:17 PDT, WebKit Review Bot 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Bruno Abinader (history only)
Comment 1 2012-07-30 10:50:59 PDT
Created attachment 155322 [details] Patch
WebKit Review Bot
Comment 2 2012-07-30 12:17:10 PDT
Comment on attachment 155322 [details] Patch Attachment 155322 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13386544 New failing tests: fast/css/first-line-text-decoration-inherited-from-parent.html fast/selectors/visited-descendant.html svg/text/text-deco-01-b.svg svg/batik/text/textDecoration2.svg css1/basic/inheritance.html fast/ruby/text-decoration-in-descendants-ruby.html svg/W3C-SVG-1.1/text-deco-01-b.svg svg/batik/text/textDecoration.svg fast/css/child-style-can-override-visited-style.html css2.1/20110323/c543-txt-decor-000.html
WebKit Review Bot
Comment 3 2012-07-30 12:17:14 PDT
Created attachment 155335 [details] Archive of layout-test-results from gce-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Bruno Abinader (history only)
Comment 4 2012-07-30 18:07:18 PDT
Comment on attachment 155322 [details] Patch Hmm, seems that I completely misunderstoond the concept here, thus I haven't attached this part of the patch into bug 90959... removing review request.
Bruno Abinader (history only)
Comment 5 2012-07-31 19:17:21 PDT
Actually not quite fully invalid :) I just found a breach while implementing patch for -webkit-text-decoration-color (bug 91638) where textDecorationsInEffect() is saner than textDecoration() usage: inside RenderObject::getTextDecorationColors, where colors for underline, overline and linethrough are obtained. However, there is no sense in having a separate patch for this change, which makes more sense to be part of that patch.
Note You need to log in before you can comment on or make changes to this bug.