Bug 230431 – RenderListMarker::imageChanged RenderBox image handling

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Gabriel Nava Marino]

RenderListMarker is a RenderBox. RenderBox is a RenderElement which can have image updates from style changes (i.e. RenderStyle has maskBoxImage).

Thus the following comment is outdated and no longer applies:
    // A list marker can't have a background or border image, so no need to call the base class method.
We should call the base class imageChanged method, which includes additional logic for handling RenderBox image updates, and handle image changes appropriately in RenderListMarker.

[Gabriel Nava Marino]

Created attachment 438528 [details]
Patch

[Darin Adler]

Comment on attachment 438528 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        No new tests (OOPS!).

Why no tests? Seems like we can and should make a test for whatever problem this caused.

> Source/WebCore/rendering/RenderListMarker.cpp:1803
> +    if (m_image) {
> +        if (o == m_image->data()) {

Let’s not nest this so much:

    if (m_image && o == m_image->data()) {
        ...

[Gabriel Nava Marino]

Created attachment 438770 [details]
Patch

[Gabriel Nava Marino]

(In reply to Darin Adler from comment #2)
> Comment on attachment 438528 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438528&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        No new tests (OOPS!).
> 
> Why no tests? Seems like we can and should make a test for whatever problem
> this caused.

I have added a new test to help catch this issue.

> 
> > Source/WebCore/rendering/RenderListMarker.cpp:1803
> > +    if (m_image) {
> > +        if (o == m_image->data()) {
> 
> Let’s not nest this so much:
> 
>     if (m_image && o == m_image->data()) {
>         ...

I have updated the patch to remove the unnecessary level of nesting.

Thank you for your feedback Darin!

[Gabriel Nava Marino]

Created attachment 438844 [details]
Patch

[Gabriel Nava Marino]

Created attachment 438848 [details]
Patch

[Gabriel Nava Marino]

Created attachment 438873 [details]
Patch

[EWS]

Committed r282880 (242009@main): <https://commits.webkit.org/242009@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438873 [details].

[Radar WebKit Bug Importer]

<rdar://problem/83406263>