Bug 258491 - Flaky background-position test and on window resize fail
Summary: Flaky background-position test and on window resize fail
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-06-24 06:41 PDT by Ahmad Saleem
Modified: 2023-07-01 06:42 PDT (History)
6 users (show)

See Also:


Attachments
Missing paint (309 bytes, text/html)
2023-06-29 17:24 PDT, zalan
no flags Details
Incorrect position (324 bytes, text/html)
2023-06-29 17:24 PDT, zalan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-06-24 06:41:08 PDT
Hi Team,

While going through some Blink merge, I noticed that my patch affected below three tests, while investigating, I noticed that we 'PASS' them but upon checking, noticed that if we resize the browser, it start showing 'red' strip, which means, we fail later on resize.

https://wpt.fyi/results/css/css-writing-modes?label=master&label=experimental&aligned&q=background-position-vrl-0

Live Test Line (Sample): http://wpt.live/css/css-writing-modes/background-position-vrl-020.xht

^ Open this in Safari Technology Preview 172 and resize browser window, we will now have 'red' strip.

__________

I did this patch here:

Link: https://github.com/WebKit/WebKit/blob/d5918187d18a309512bf04da9a709f43762634a0/Source/WebCore/rendering/BackgroundPainter.cpp#L554

LayoutRect extendedBackgroundRect = view.frameView().extendedBackgroundRectForPainting();
+                if (renderer.style().writingMode() == WritingMode::RightToLeft)
+                   left = borderBoxRect.width() - positioningAreaSize.width() - right - renderer.marginRight();
+               else

__________

Blink Commit: https://src.chromium.org/viewvc/blink?view=revision&revision=194132

________

Just wanted to raise, so we can track these failures.

Thanks!
Comment 1 Ahmad Saleem 2023-06-24 06:44:36 PDT
NOTE - I did patch with and without this change and without change, we fail the test and on resize, we will have 'red' strip.
Comment 2 zalan 2023-06-29 17:24:28 PDT
Created attachment 466877 [details]
Missing paint

I think we've got multiple bugs here:
- missing initial paint when image becomes available
- incorrect position on vertical-rl body/html
Comment 3 zalan 2023-06-29 17:24:42 PDT
Created attachment 466878 [details]
Incorrect position
Comment 4 zalan 2023-06-29 17:25:49 PDT
(In reply to Ahmad Saleem from comment #0)
> Hi Team,
> 
> While going through some Blink merge, I noticed that my patch affected below
> three tests, while investigating, I noticed that we 'PASS' them but upon
> checking, noticed that if we resize the browser, it start showing 'red'
> strip, which means, we fail later on resize.
> 
> https://wpt.fyi/results/css/css-writing-
> modes?label=master&label=experimental&aligned&q=background-position-vrl-0
> 
> Live Test Line (Sample):
> http://wpt.live/css/css-writing-modes/background-position-vrl-020.xht
> 
> ^ Open this in Safari Technology Preview 172 and resize browser window, we
> will now have 'red' strip.
> 
> __________
> 
> I did this patch here:
> 
> Link:
> https://github.com/WebKit/WebKit/blob/
> d5918187d18a309512bf04da9a709f43762634a0/Source/WebCore/rendering/
> BackgroundPainter.cpp#L554
> 
> LayoutRect extendedBackgroundRect =
> view.frameView().extendedBackgroundRectForPainting();
> +                if (renderer.style().writingMode() ==
> WritingMode::RightToLeft)
> +                   left = borderBoxRect.width() -
> positioningAreaSize.width() - right - renderer.marginRight();
> +               else
> 
> __________
> 
> Blink Commit:
> https://src.chromium.org/viewvc/blink?view=revision&revision=194132
> 
> ________
> 
> Just wanted to raise, so we can track these failures.
> 
> Thanks!
The second case fails with Chrome too so I assume the fix is not sufficient.
Comment 5 Ahmad Saleem 2023-06-29 17:26:58 PDT
(In reply to zalan from comment #4)
> (In reply to Ahmad Saleem from comment #0)
> > Hi Team,
> > 
> > While going through some Blink merge, I noticed that my patch affected below
> > three tests, while investigating, I noticed that we 'PASS' them but upon
> > checking, noticed that if we resize the browser, it start showing 'red'
> > strip, which means, we fail later on resize.
> > 
> > https://wpt.fyi/results/css/css-writing-
> > modes?label=master&label=experimental&aligned&q=background-position-vrl-0
> > 
> > Live Test Line (Sample):
> > http://wpt.live/css/css-writing-modes/background-position-vrl-020.xht
> > 
> > ^ Open this in Safari Technology Preview 172 and resize browser window, we
> > will now have 'red' strip.
> > 
> > __________
> > 
> > I did this patch here:
> > 
> > Link:
> > https://github.com/WebKit/WebKit/blob/
> > d5918187d18a309512bf04da9a709f43762634a0/Source/WebCore/rendering/
> > BackgroundPainter.cpp#L554
> > 
> > LayoutRect extendedBackgroundRect =
> > view.frameView().extendedBackgroundRectForPainting();
> > +                if (renderer.style().writingMode() ==
> > WritingMode::RightToLeft)
> > +                   left = borderBoxRect.width() -
> > positioningAreaSize.width() - right - renderer.marginRight();
> > +               else
> > 
> > __________
> > 
> > Blink Commit:
> > https://src.chromium.org/viewvc/blink?view=revision&revision=194132
> > 
> > ________
> > 
> > Just wanted to raise, so we can track these failures.
> > 
> > Thanks!
> The second case fails with Chrome too so I assume the fix is not sufficient.

Indeed, just tested on Chrome Canary 117 and only Firefox Nightly 116 is passing it.

Thanks for your input as always.
Comment 6 Radar WebKit Bug Importer 2023-07-01 06:42:16 PDT
<rdar://problem/111614474>