Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebKit ignores the nowrap on td nowrap="nowrap", if an absolute width is specified #4114

Closed
wants to merge 5 commits into from

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Sep 7, 2022

WebKit ignores the nowrap on td nowrap="nowrap", if an absolute width is specified

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

Merge - https://chromium.googlesource.com/chromium/src/+/5e6c851254f90f6f495713f7248ffe9a155de413

Previously we only applied it when a 'td' element had no fixed-width. This was incorrect behavior and it will align Webkit with Blink and Gecko for Web interop.

* Source/WebCore/html/HTMLTableCellElement.cpp (HTMLTableCellElement::hasPresentationalHintsForAttribute) - Update nowrap Attribute to respect CSSValueNowrap rather than CSSValueWebkitNowrap
* Source/WebCore/style/StyleAdjuster.cpp (Adjuster:adjust) - Removed logic about td tag only accepting wrap for Fixed Width
* LayoutTests/imported/w3c/web-platform-tests/quirks/table-cell-nowrap-minimum-width-calculation-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/003-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/003-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/tables/021-expected.png - Updated Test Expectations
* LayoutTests/platform/glib/fast/tables/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/rowspan-paint-order-vertical-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug57828-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug78162-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug78162-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug92143-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/fast/table/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug78162-expected.txt - Updated Tests Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug92143-expected.txt - Updated Tests Expectations
* LayoutTests/platform/mac/fast/table/003-expected.png - Updated Tests Expectations
* LayoutTests/platform/mac/fast/table/003-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/021-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug57828-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug78162-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug78162-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug92143-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug92143-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/rowspan-paint-order-vertical-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/glib/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations

e990887

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style ❌ πŸ›  ios ❌ πŸ›  mac ❌ πŸ›  wpe   πŸ›  πŸ§ͺ win
❌ πŸ›  ios-sim ❌ πŸ›  mac-debug ❌ πŸ›  gtk ❌ πŸ›  wincairo
❌ πŸ§ͺ webkitperl ❌ πŸ§ͺ ios-wk2 ❌ πŸ›  mac-AS-debug ❌ πŸ§ͺ gtk-wk2
❌ πŸ§ͺ api-ios ❌ πŸ§ͺ api-mac ❌ πŸ§ͺ api-gtk
❌ πŸ›  tv ❌ πŸ§ͺ mac-wk1
❌ πŸ›  tv-sim ❌ πŸ§ͺ mac-wk2
❌ πŸ›  watch ❌ πŸ§ͺ mac-AS-debug-wk2
❌ πŸ›  watch-sim ❌ πŸ§ͺ mac-wk2-stress

Deleting a button element leaves the style inside the button element

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

Reviewed by NOBODY (OOPS!).

Patch Authored by - Sukolsak Sakshuwong

Make the DeleteSelectionCommand save typing style only for formattable nodes according to the HTML Editing specification and match "Blink" behavior.

* LayoutTests/editing/deleting/delete-button.html
* LayoutTests/editing/deleting/delete-button-expected.txt
* Source/WebCore/editing/DelectionSelectionCommand.cpp
(WebCore::DeleteSelectionCommand::saveTypingStyleState)
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 8, 2022
@nt1m nt1m requested a review from anttijk September 8, 2022 04:05
Copy link
Contributor

@alanbaradlay alanbaradlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what CSSValueWebkitNowrap is used for (WhiteSpace::KHTMLNoWrap) but a simpler fix would be just to call setWhiteSpace(NoWrap) unconditionally in the adjuster.
(It's probably not super relevant anymore, but someone who knows about KHTMLNoWrap should comment on it).

@Ahmad-S792
Copy link
Contributor Author

I am not sure what CSSValueWebkitNowrap is used for (WhiteSpace::KHTMLNoWrap) but a simpler fix would be just to call setWhiteSpace(NoWrap) unconditionally in the adjuster.
(It's probably not super relevant anymore, but someone who knows about KHTMLNoWrap should comment on it).

I can only find two places "CSSValueWebkitNowrap" is present in "CSSPrimitiveValueMappings.h" and IMO we can remove that as well because other is about the use in "HTMLTableCellElement", which I am getting rid of with this patch.

Ahmad-S792 and others added 3 commits September 10, 2022 20:25
… is specified

WebKit ignores the nowrap on td nowrap="nowrap", if an absolute width is specified

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

Merge - https://chromium.googlesource.com/chromium/src/+/5e6c851254f90f6f495713f7248ffe9a155de413

Previously we only applied it when a <td> element had no fixed-width. This was incorrect behavior and it will align Webkit with Blink and Gecko for Web interop.

* Source/WebCore/html/HTMLTableCellElement.cpp
(HTMLTableCellElement::hasPresentationalHintsForAttribute) - Update nowrap Attribute to respect CSSValueNowrap rather than CSSValueWebkitNowrap
* Source/WebCore/style/StyleAdjuster.cpp
(Adjuster:adjust) - Removed logic about td tag only accepting wrap for Fixed Width
* LayoutTests/imported/w3c/web-platform-tests/quirks/table-cell-nowrap-minimum-width-calculation-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/003-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/003-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/tables/021-expected.png - Updated Test Expectations
* LayoutTests/platform/glib/fast/tables/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/rowspan-paint-order-vertical-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug57828-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug78162-expected.png - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug78162-expected.txt - Updated Test Expectations
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug92143-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/fast/table/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug78162-expected.txt - Updated Tests Expectations
* LayoutTests/platform/ios/tables/mozilla/bugs/bug92143-expected.txt - Updated Tests Expectations
* LayoutTests/platform/mac/fast/table/003-expected.png - Updated Tests Expectations
* LayoutTests/platform/mac/fast/table/003-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/021-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/021-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug57828-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug78162-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug78162-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug92143-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/tables/mozilla/bugs/bug92143-expected.txt - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/rowspan-paint-order-vertical-expected.png - Updated Test Expectations
* LayoutTests/platform/mac/fast/table/rowspan-paint-order-vertical-expected.txt - Updated Test Expectations
* LayoutTests/platform/glib/mozilla/bugs/bug57828-expected.txt - Updated Test Expectations
@Ahmad-S792 Ahmad-S792 closed this Sep 11, 2022
@Ahmad-S792 Ahmad-S792 deleted the fix183642 branch September 17, 2022 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging-blocked Applied to prevent a change from being merged
Projects
None yet
4 participants