Issue 468503003: Remove FIXME in StyleResolver::styleForElement

Issue 468503003: Remove FIXME in StyleResolver::styleForElement (Closed)

Created:
6 years, 4 months ago by rwlbuis

Modified:
6 years, 4 months ago

Reviewers:
esprehn

CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-rendering, dglazkov+blink, eae+blinkwatch, ed+blinkwatch_opera.com, jchaffraix+rendering, leviw+renderwatch, pdr., rune+blink, zoltan1

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Project:
blink

Visibility:
Public.

More Reviews

Description

Remove FIXME in StyleResolver::styleForElement The action suggested in the FIXME has the downside of not doing the right thing when the body has display set to none, as the test shows. So remove the FIXME and keep the old behavior. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180478

Patch Set 1 #

Patch Set 2 : Add test #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+30 lines, -1 line)			Patch
A	LayoutTests/fast/css/webkit-text-display-none.html	View	1	1 chunk	+20 lines, -0 lines	0 comments	Download
A	LayoutTests/fast/css/webkit-text-display-none-expected.txt	View	1	1 chunk	+10 lines, -0 lines	0 comments	Download
M	Source/core/css/resolver/StyleResolver.cpp	View	1	1 chunk	+0 lines, -1 line	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

esprehn

This doesn't work, if everything is display: none then getComputedStyle() on a descendant of <body> ...

6 years, 4 months ago (2014-08-12 23:35:34 UTC) #2

rwlbuis

On 2014/08/12 23:35:34, esprehn wrote: > This doesn't work, if everything is display: none then ...

6 years, 4 months ago (2014-08-13 17:51:12 UTC) #3

rwlbuis

On 2014/08/13 17:51:12, rwlbuis wrote: > On 2014/08/12 23:35:34, esprehn wrote: > > This doesn't ...

6 years, 4 months ago (2014-08-13 21:30:24 UTC) #4

esprehn

On 2014/08/13 at 21:30:24, rob.buis wrote: > ..e > > Ok forget above, I see ...

6 years, 4 months ago (2014-08-16 08:01:59 UTC) #5

rwlbuis

On 2014/08/16 08:01:59, esprehn wrote: > On 2014/08/13 at 21:30:24, rob.buis wrote: > > ..e ...

6 years, 4 months ago (2014-08-18 15:41:48 UTC) #6

esprehn

lgtm, but please fix the patch title/description before commiting.

6 years, 4 months ago (2014-08-18 16:36:44 UTC) #7

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rob.buis@samsung.com/468503003/20001

6 years, 4 months ago (2014-08-18 17:02:31 UTC) #9

commit-bot: I haz the power

6 years, 4 months ago (2014-08-18 17:49:56 UTC) #10

Message was sent while issue was closed.

Committed patchset #2 (20001) as 180478

Expand Messages | Collapse Messages