Issue 351033003: Simplify use of styleOrFirstLineStyle

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 351033003: Simplify use of styleOrFirstLineStyle (Closed)

Created:
6 years, 6 months ago by davve

Modified:
5 years, 6 months ago

Reviewers:
abarth-chromium, esprehn, eseidel

CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr., rune+blink, zoltan1

Base URL:
https://chromium.googlesource.com/chromium/blink.git@rename-style-bool-to-0

Project:
blink

Visibility:
Public.

More Reviews

Description

Simplify use of styleOrFirstLineStyle One case is when the parameter to styleOrFirstLineStyle is hardcoded, we can then just replace the call with what it expands to. The other case is when code checks document().styleEngine()->usesFirstLineRules() before calling styleOrFirstLineStyle(), firstLineStyle() itself checks document().styleEngine()->usesFirstLineRules() so the check is redundant.

Patch Set 1 #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+6 lines, -14 lines)			Patch
M	Source/core/rendering/RenderBR.cpp	View	1 chunk	+1 line, -2 lines	0 comments	Download
M	Source/core/rendering/RenderBlock.cpp	View	2 chunks	+2 lines, -3 lines	0 comments	Download
M	Source/core/rendering/RenderInline.cpp	View	2 chunks	+3 lines, -9 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

abarth-chromium

These changes seem reasonable, but I'm not the right reviewer. +eseidel,esprehn

6 years, 6 months ago (2014-06-25 19:49:27 UTC) #2

esprehn

I'd rather you added an enum and kept style(FirstLineStyle) or Style(OwnStyle) or something like that. ...

6 years, 6 months ago (2014-06-25 20:14:32 UTC) #3

davve

6 years, 6 months ago (2014-06-26 13:34:07 UTC) #4

On 2014/06/25 20:14:32, esprehn wrote:
> I'd rather you added an enum and kept style(FirstLineStyle) or Style(OwnStyle)
> or something like that. The bool argument is silly, which side does it mean?

I made https://codereview.chromium.org/351213002/ as a POC. Still not happy with
the name of the enum though, StyleSet feels a little bit too generic, but the
best one I could come up with for the moment.

Expand Messages | Collapse Messages