Bug 257082
| Summary: | Changing 'DisplayType::InlineGrid' and 'DisplayType::InlineFlex' to output properly in StyleAdjuster.cpp | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | bfulgham, koivisto, ntim, simon.fraser, webkit-bug-importer, zalan |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar, WPTImpact |
| Version: | Safari Technology Preview | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Ahmad Saleem
Hi Team,
While trying to fix another bug, I came across another fix to failing testcase for following WPT test:
WPT Test: legend-grid-flex-multicol.html & legend-display.html
WPT Test Link (Live): http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/legend-grid-flex-multicol.html
^ Make us pass last two remaining failing tests.
WPT Test: legend-display.html
WPT Test Link (Live): http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/legend-display.html
^ Make us pass more 8 tests cases.
________________________
WebKit Source to change:
https://github.com/WebKit/WebKit/blob/e17c2153ab908a5821a2b72ee9fe7856b98aac12/Source/WebCore/style/StyleAdjuster.cpp#L116
^ Change above to following (Partial Copy - diff):
case DisplayType::Block:
case DisplayType::Table:
case DisplayType::Box:
case DisplayType::FlowRoot:
case DisplayType::ListItem:
return display;
case DisplayType::InlineTable:
return DisplayType::Table;
case DisplayType::InlineBox:
return DisplayType::InlineBox;
case DisplayType::Flex:
return DisplayType::Flex;
case DisplayType::InlineFlex:
return DisplayType::InlineFlex;
case DisplayType::Grid:
return DisplayType::Grid;
case DisplayType::InlineGrid:
return DisplayType::InlineGrid;
case DisplayType::Inline:
case DisplayType::InlineBlock:
_____________________________________
^ Just wanted to raise so we can look into it.
Thanks!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Ahmad Saleem
(In reply to Ahmad Saleem from comment #0)
> Hi Team,
>
> While trying to fix another bug, I came across another fix to failing
> testcase for following WPT test:
>
> WPT Test: legend-grid-flex-multicol.html & legend-display.html
>
> WPT Test Link (Live):
> http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> elements/legend-grid-flex-multicol.html
>
> ^ Make us pass last two remaining failing tests.
>
> WPT Test: legend-display.html
>
> WPT Test Link (Live):
> http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> elements/legend-display.html
>
> ^ Make us pass more 8 tests cases.
>
> ________________________
>
> WebKit Source to change:
>
> https://github.com/WebKit/WebKit/blob/
> e17c2153ab908a5821a2b72ee9fe7856b98aac12/Source/WebCore/style/StyleAdjuster.
> cpp#L116
>
> ^ Change above to following (Partial Copy - diff):
>
> case DisplayType::Block:
> case DisplayType::Table:
> case DisplayType::Box:
> case DisplayType::FlowRoot:
> case DisplayType::ListItem:
> return display;
> case DisplayType::InlineTable:
> return DisplayType::Table;
> case DisplayType::InlineBox:
> return DisplayType::InlineBox;
> case DisplayType::Flex:
> return DisplayType::Flex;
> case DisplayType::InlineFlex:
> return DisplayType::InlineFlex;
> case DisplayType::Grid:
> return DisplayType::Grid;
> case DisplayType::InlineGrid:
> return DisplayType::InlineGrid;
> case DisplayType::Inline:
> case DisplayType::InlineBlock:
>
> _____________________________________
> ^ Just wanted to raise so we can look into it.
>
> Thanks!
Although it regresses:
css-display/parsing/display-computed.html
css-flexbox/flexbox_inline-float.html
css-flexbox/percentage-padding-002.html
^ Might be something else. We might need something specific, just for 'legend' attribute in StyleAdjuster.cpp'.
alan
(In reply to Ahmad Saleem from comment #0)
> Hi Team,
>
> While trying to fix another bug, I came across another fix to failing
> testcase for following WPT test:
>
> WPT Test: legend-grid-flex-multicol.html & legend-display.html
>
> WPT Test Link (Live):
> http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> elements/legend-grid-flex-multicol.html
>
> ^ Make us pass last two remaining failing tests.
>
> WPT Test: legend-display.html
>
> WPT Test Link (Live):
> http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> elements/legend-display.html
>
> ^ Make us pass more 8 tests cases.
>
> ________________________
>
> WebKit Source to change:
>
> https://github.com/WebKit/WebKit/blob/
> e17c2153ab908a5821a2b72ee9fe7856b98aac12/Source/WebCore/style/StyleAdjuster.
> cpp#L116
>
> ^ Change above to following (Partial Copy - diff):
>
> case DisplayType::Block:
> case DisplayType::Table:
> case DisplayType::Box:
> case DisplayType::FlowRoot:
> case DisplayType::ListItem:
> return display;
> case DisplayType::InlineTable:
> return DisplayType::Table;
> case DisplayType::InlineBox:
> return DisplayType::InlineBox;
> case DisplayType::Flex:
> return DisplayType::Flex;
> case DisplayType::InlineFlex:
> return DisplayType::InlineFlex;
> case DisplayType::Grid:
> return DisplayType::Grid;
> case DisplayType::InlineGrid:
> return DisplayType::InlineGrid;
> case DisplayType::Inline:
> case DisplayType::InlineBlock:
>
> _____________________________________
> ^ Just wanted to raise so we can look into it.
>
> Thanks!
equivalentBlockDisplay (as the name implies) is supposed to return the block version of an inline type of value.
> case DisplayType::InlineFlex:
> return DisplayType::InlineFlex;
would certainly break this contract.
Ahmad Saleem
(In reply to zalan from comment #2)
> (In reply to Ahmad Saleem from comment #0)
> > Hi Team,
> >
> > While trying to fix another bug, I came across another fix to failing
> > testcase for following WPT test:
> >
> > WPT Test: legend-grid-flex-multicol.html & legend-display.html
> >
> > WPT Test Link (Live):
> > http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> > elements/legend-grid-flex-multicol.html
> >
> > ^ Make us pass last two remaining failing tests.
> >
> > WPT Test: legend-display.html
> >
> > WPT Test Link (Live):
> > http://wpt.live/html/rendering/non-replaced-elements/the-fieldset-and-legend-
> > elements/legend-display.html
> >
> > ^ Make us pass more 8 tests cases.
> >
> > ________________________
> >
> > WebKit Source to change:
> >
> > https://github.com/WebKit/WebKit/blob/
> > e17c2153ab908a5821a2b72ee9fe7856b98aac12/Source/WebCore/style/StyleAdjuster.
> > cpp#L116
> >
> > ^ Change above to following (Partial Copy - diff):
> >
> > case DisplayType::Block:
> > case DisplayType::Table:
> > case DisplayType::Box:
> > case DisplayType::FlowRoot:
> > case DisplayType::ListItem:
> > return display;
> > case DisplayType::InlineTable:
> > return DisplayType::Table;
> > case DisplayType::InlineBox:
> > return DisplayType::InlineBox;
> > case DisplayType::Flex:
> > return DisplayType::Flex;
> > case DisplayType::InlineFlex:
> > return DisplayType::InlineFlex;
> > case DisplayType::Grid:
> > return DisplayType::Grid;
> > case DisplayType::InlineGrid:
> > return DisplayType::InlineGrid;
> > case DisplayType::Inline:
> > case DisplayType::InlineBlock:
> >
> > _____________________________________
> > ^ Just wanted to raise so we can look into it.
> >
> > Thanks!
> equivalentBlockDisplay (as the name implies) is supposed to return the block
> version of an inline type of value.
> > case DisplayType::InlineFlex:
> > return DisplayType::InlineFlex;
> would certainly break this contract.
Creating separate function for 'Legend' & 'Fieldset' and then using it?
Radar WebKit Bug Importer
<rdar://problem/109923265>
Ahmad Saleem
Creating another 'Static' function in StyleAdjuster.cpp:
static DisplayType equivalentDisplayForLegend(const RenderStyle& style)
{
switch (auto display = style.display()) {
case DisplayType::Block:
case DisplayType::Table:
case DisplayType::Box:
case DisplayType::FlowRoot:
case DisplayType::ListItem:
return display;
case DisplayType::InlineTable:
return DisplayType::Table;
case DisplayType::InlineBox:
return DisplayType::InlineBox;
case DisplayType::Flex:
return DisplayType::Flex;
case DisplayType::InlineFlex:
return DisplayType::InlineFlex;
case DisplayType::Grid:
return DisplayType::Grid;
case DisplayType::InlineGrid:
return DisplayType::InlineGrid;
case DisplayType::Inline:
case DisplayType::InlineBlock:
case DisplayType::TableRowGroup:
case DisplayType::TableHeaderGroup:
case DisplayType::TableFooterGroup:
case DisplayType::TableRow:
case DisplayType::TableColumnGroup:
case DisplayType::TableColumn:
case DisplayType::TableCell:
case DisplayType::TableCaption:
return DisplayType::Block;
case DisplayType::Contents:
ASSERT_NOT_REACHED();
return DisplayType::Contents;
case DisplayType::None:
ASSERT_NOT_REACHED();
return DisplayType::None;
}
ASSERT_NOT_REACHED();
return DisplayType::Block;
}
and then modifying Line 363 to:
style.setEffectiveDisplay(equivalentDisplayForLegend(style));
__________________
@Antti , @Tim , @Alan - any suggestion? It will fix some of these tests.