[Webkit-unassigned] [Bug 192980] [css-lists] Fix list marker issues related to line breaks and markers disappearance

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=192980

Javier Fernandez <jfernandez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jfernandez at igalia.com

--- Comment #64 from Javier Fernandez <jfernandez at igalia.com> ---
Comment on attachment 358881
  --> https://bugs.webkit.org/attachment.cgi?id=358881
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:519
> +        if (logicalHeightLength.isFixed() && isAnonymous() && parent() && is<RenderListItem>(parent())) {

is<> already checks for nullptr, so there is no need for the parent() clause.

> Source/WebCore/rendering/RenderBlock.cpp:521
> +            if (child && is<RenderListMarker>(child) && !child->nextSibling())

ditto

> Source/WebCore/rendering/RenderBlock.h:-354
> -    Optional<int> firstLineBaseline() const override;

Why we need to change visibility of this function ? As far as I understood, m_baselineProvides is a RenderBlock, so protected visibility should be enough.

> Source/WebCore/rendering/RenderListItem.cpp:461
> +    if (baselineProvider && is<RenderBlockFlow>(baselineProvider) && downcast<RenderBlockFlow>(baselineProvider)->firstRootBox() == &markerRoot)

No need to check for nullptr here if we already use the is<> template.

> Source/WebCore/rendering/RenderListItem.cpp:465
> +    if (!backToOriginalBaseline)

Why not doing it this way instead ? 

 Optional<int> offset = backToOriginalBaseline ? nullopt : baselineProvider->firstLineBaseline();

Then in the if below we can just have one clause:

   if (!offset) {

> Source/WebCore/rendering/RenderListItem.cpp:474
> +    if (offset) {

Maybe we can early return here in case of !offset instead.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190111/41046472/attachment-0001.html>