Bug 252511

Summary:	Distribute all extra height to rows
Product:	WebKit	Reporter:	Ahmad Saleem <ahmad.saleem792>
Component:	Tables	Assignee:	Nobody <webkit-unassigned>
Status:	NEW ---
Severity:	Normal	CC:	webkit-bug-importer, zalan
Priority:	P2	Keywords:	InRadar
Version:	Safari Technology Preview
Hardware:	Unspecified
OS:	Unspecified

Description Ahmad Saleem 2023-02-17 17:01:05 PST

Hi Team,

This is commit, which can be merged but it mostly addresses FIXME in WebKit rather than fixing any failing test case.

Blink Commit - https://chromium.googlesource.com/chromium/src.git/+/d22a0616e628fe5e88618b288a98844b6891671c

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderTable.cpp#414 & https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderTableSection.cpp#433

It compiles and I have tested locally but it does not seem to progress any WPT test cases like these - https://wpt.fyi/results/css/css-tables/height-distribution?label=master&label=experimental&aligned&view=subtest&q=extra%20heigh

So I just wanted to raise to get input whether we should merge this to get rid of FIXME or we can ignore it.

Thanks!

Comment 1 zalan 2023-02-17 20:50:25 PST

(In reply to Ahmad Saleem from comment #0)
> Hi Team,
> 
> This is commit, which can be merged but it mostly addresses FIXME in WebKit
> rather than fixing any failing test case.
> 
> Blink Commit -
> https://chromium.googlesource.com/chromium/src.git/+/
> d22a0616e628fe5e88618b288a98844b6891671c
> 
> WebKit Source -
> https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderTable.
> cpp#414 &
> https://searchfox.org/wubkat/source/Source/WebCore/rendering/
> RenderTableSection.cpp#433
> 
> It compiles and I have tested locally but it does not seem to progress any
> WPT test cases like these -
> https://wpt.fyi/results/css/css-tables/height-
> distribution?label=master&label=experimental&aligned&view=subtest&q=extra%20h
> eigh
> 
> So I just wanted to raise to get input whether we should merge this to get
> rid of FIXME or we can ignore it.
> 
> Thanks!
The int -> float change surely fixes some height distribution bugs. I'd say merge away!

Comment 2 Radar WebKit Bug Importer 2023-02-24 17:02:17 PST

<rdar://problem/105907566>

Comment 3 Ahmad Saleem 2023-05-28 15:11:18 PDT

I tried this with PR: https://github.com/WebKit/WebKit/pull/10329

and but I crashed a lot of 'tables' test with assertion failure but when I tried 'build-webkit --release' by also adding this commit:
https://chromium.googlesource.com/chromium/src.git/+/9c14979722d3761c5449f3ef9f68bd2fee4f82a0

and did:

'run-webkit-tests' on fast/table/ and css/css-tables on WPT suite, I don't get any other failures or crashes.

Is 'build-webkit --debug' different from 'mac-AS-debug-wk2'?

Or should I try again with above mention commit as well?

Comment 4 Ahmad Saleem 2023-05-28 15:15:07 PDT

(In reply to Ahmad Saleem from comment #3)
> I tried this with PR: https://github.com/WebKit/WebKit/pull/10329
> 
> and but I crashed a lot of 'tables' test with assertion failure but when I
> tried 'build-webkit --release' by also adding this commit:
> https://chromium.googlesource.com/chromium/src.git/+/
> 9c14979722d3761c5449f3ef9f68bd2fee4f82a0
> 
> and did:
> 
> 'run-webkit-tests' on fast/table/ and css/css-tables on WPT suite, I don't
> get any other failures or crashes.
> 
> Is 'build-webkit --debug' different from 'mac-AS-debug-wk2'?
> 
> Or should I try again with above mention commit as well?

Sorry - I tried 'build-webkit --debug' and it didn't crash any of above tests.