Remove support for the hspace and vspace on table #3573

Ahmad-S792 · 2022-08-23T11:18:50Z

Remove support for the hspace and vspace attributes on table
https://bugs.webkit.org/show_bug.cgi?id=120133

Reviewed by NOBODY (OOPS!).

Partial Merge - Since Webkit does not use "User Counter" - https://src.chromium.org/viewvc/blink?view=revision&revision=195601

It will align Webkit with Blink, Gecko and Web-spec.

* LayoutTests/imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/tables/table-vspace-hspace-expected.txt: Updated Expectations to 'PASS'.
* LayoutTests/imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/tables/table-vspace-hspace-s-expected.txt: Updated Expectations to 'PASS'.
* LayoutTests/imported/w3c/web-platform-tests/html/rendering/unmapped-attributes-expected.txt: Updated Expectations to 'PASS'.
* LayoutTests/platform/glib/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable.
* LayoutTests/platform/gtk/tables/mozilla/bugs/bug1261-expected.png: Updated based on new expectations.
* LayoutTests/platform/ios/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable.
* LayoutTests/platform/mac/tables/mozilla/bugs/bug1261-expected.png: Updated based on new expectations.
* LayoutTests/platform/mac/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable.
* Source/WebCore/html/HTMLTableElement.cpp
(HTMLTableElement::collectPresentationalHintsForAttribute): Removed 'hspace' and 'vspace' attributes on table to match with all other browsers.

7a8b822

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 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

webkit-early-warning-system · 2022-08-23T11:19:01Z

EWS run on previous version of this PR (hash 10aa0c5)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 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~~

Ahmad-S792 · 2022-08-23T13:59:09Z

@rniwa - Is "bug1261" test case required? I fixed all other tests on local repo and can push and squash the commit? Appreciate if you can confirm and I will do the needful. Thanks!

webkit-early-warning-system · 2022-08-23T17:00:53Z

EWS run on previous version of this PR (hash e3fa5fb)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	~~🛠 🧪 win~~
⏳ 🧪 bindings	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	~~🧪 api-ios~~	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

Test Case fixes

webkit-early-warning-system · 2022-08-23T17:02:34Z

EWS run on previous version of this PR (hash 534ada2)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 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
❌ 🛠 🧪 merge	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
❌ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

rniwa

r=me

Ahmad-S792 · 2022-08-23T19:41:57Z

@rniwa Will merge-queue and Labels be added automatically? Since I don't have access and I am not committer, so I am not sure.

rniwa · 2022-08-23T19:43:30Z

@rniwa Will merge-queue and Labels be added automatically? Since I don't have access and I am not committer, so I am not sure.

No, I need to add it. I'm waiting for Mac EWS bots' results.

webkit-commit-queue · 2022-08-24T00:11:46Z

No reviewer information in commit message, blocking PR #3573

rniwa

r=me

webkit-commit-queue · 2022-08-24T00:15:50Z

No reviewer information in commit message, blocking PR #3573

JonWBedard · 2022-08-24T00:49:49Z

@Ahmad-S792 Your PR description is great, that should be your commit message, that's what merge-queue is complaining about

Remove support for the hspace and vspace attributes on table https://bugs.webkit.org/show_bug.cgi?id=120133 Reviewed by NOBODY (OOPS!). Partial Merge - Since Webkit does not use "User Counter" - https://src.chromium.org/viewvc/blink?view=revision&revision=195601 It will align Webkit with Blink, Gecko and Web-spec. * LayoutTests/imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/tables/table-vspace-hspace-expected.txt: Updated Expectations to 'PASS'. * LayoutTests/imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/tables/table-vspace-hspace-s-expected.txt: Updated Expectations to 'PASS'. * LayoutTests/imported/w3c/web-platform-tests/html/rendering/unmapped-attributes-expected.txt: Updated Expectations to 'PASS'. * LayoutTests/platform/glib/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable. * LayoutTests/platform/gtk/tables/mozilla/bugs/bug1261-expected.png: Updated based on new expectations. * LayoutTests/platform/ios/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable. * LayoutTests/platform/mac/tables/mozilla/bugs/bug1261-expected.png: Updated based on new expectations. * LayoutTests/platform/mac/tables/mozilla/bugs/bug1261-expected.txt: Rebased RenderTable. * Source/WebCore/html/HTMLTableElement.cpp (HTMLTableElement::collectPresentationalHintsForAttribute): Removed 'hspace' and 'vspace' attributes on table to match with all other browsers.

… into fix120133

webkit-early-warning-system · 2022-08-24T01:11:26Z

EWS run on previous version of this PR (hash b95827c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 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

Ahmad-S792 · 2022-08-24T01:21:11Z

@Ahmad-S792 Your PR description is great, that should be your commit message, that's what merge-queue is complaining about

I messed up, I updated commit message via amend (I use GitHub Desktop, not CLI) and then when I tried to Pull Origin, it created three commits and I am unable to squash them. :-( Any suggestion or tips?

webkit-early-warning-system · 2022-08-24T11:38:52Z

7a8b822

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 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

JonWBedard · 2022-08-24T16:30:26Z

@Ahmad-S792 Your PR description is great, that should be your commit message, that's what merge-queue is complaining about

I messed up, I updated commit message via amend (I use GitHub Desktop, not CLI) and then when I tried to Pull Origin, it created three commits and I am unable to squash them. :-( Any suggestion or tips?

You'll need to squash and then force push, this looks like GitHub's documentation on that https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/managing-commits/squashing-commits
(I don't use GitHub Desktop, so I'm just reading through documentation knowing how git works under the hood, that should be the right thing)

Ahmad-S792 · 2022-08-24T18:42:07Z

@Ahmad-S792 Your PR description is great, that should be your commit message, that's what merge-queue is complaining about

I messed up, I updated commit message via amend (I use GitHub Desktop, not CLI) and then when I tried to Pull Origin, it created three commits and I am unable to squash them. :-( Any suggestion or tips?

You'll need to squash and then force push, this looks like GitHub's documentation on that https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/managing-commits/squashing-commits (I don't use GitHub Desktop, so I'm just reading through documentation knowing how git works under the hood, that should be the right thing)

Damn! It is giving me same error, which I showed to @rniwa on Slack Channel. I am going to close this and create a new one. :-(

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 23, 2022

Remove support for the hspace and vspace

534ada2

Test Case fixes

Ahmad-S792 marked this pull request as ready for review August 23, 2022 18:03

Ahmad-S792 requested review from cdumez and rniwa as code owners August 23, 2022 18:03

rniwa approved these changes Aug 23, 2022

View reviewed changes

rniwa added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Aug 23, 2022

webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Aug 24, 2022

rniwa approved these changes Aug 24, 2022

View reviewed changes

rniwa added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Aug 24, 2022

webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Aug 24, 2022

Ahmad-S792 added 2 commits August 24, 2022 02:10

Merge branch 'fix1201333' of https://github.com/Ahmad-S792/WebKit-Ahmad…

b95827c

… into fix120133

Merge branch 'WebKit:main' into fix1201333

7a8b822

Ahmad-S792 closed this Aug 24, 2022

Ahmad-S792 deleted the fix1201333 branch August 24, 2022 20:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove support for the hspace and vspace on table #3573

Remove support for the hspace and vspace on table #3573

Ahmad-S792 commented Aug 23, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Aug 23, 2022 •

edited

Ahmad-S792 commented Aug 23, 2022

webkit-early-warning-system commented Aug 23, 2022 •

edited

webkit-early-warning-system commented Aug 23, 2022 •

edited

rniwa left a comment

Ahmad-S792 commented Aug 23, 2022

rniwa commented Aug 23, 2022

webkit-commit-queue commented Aug 24, 2022

rniwa left a comment

webkit-commit-queue commented Aug 24, 2022

JonWBedard commented Aug 24, 2022

webkit-early-warning-system commented Aug 24, 2022 •

edited

Ahmad-S792 commented Aug 24, 2022

webkit-early-warning-system commented Aug 24, 2022 •

edited

JonWBedard commented Aug 24, 2022

Ahmad-S792 commented Aug 24, 2022

Remove support for the hspace and vspace on table #3573

Remove support for the hspace and vspace on table #3573

Conversation

Ahmad-S792 commented Aug 23, 2022 • edited by webkit-early-warning-system

webkit-early-warning-system commented Aug 23, 2022 • edited

Ahmad-S792 commented Aug 23, 2022

webkit-early-warning-system commented Aug 23, 2022 • edited

webkit-early-warning-system commented Aug 23, 2022 • edited

rniwa left a comment

Choose a reason for hiding this comment

Ahmad-S792 commented Aug 23, 2022

rniwa commented Aug 23, 2022

webkit-commit-queue commented Aug 24, 2022

rniwa left a comment

Choose a reason for hiding this comment

webkit-commit-queue commented Aug 24, 2022

JonWBedard commented Aug 24, 2022

webkit-early-warning-system commented Aug 24, 2022 • edited

Ahmad-S792 commented Aug 24, 2022

webkit-early-warning-system commented Aug 24, 2022 • edited

JonWBedard commented Aug 24, 2022

Ahmad-S792 commented Aug 24, 2022

Ahmad-S792 commented Aug 23, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Aug 23, 2022 •

edited

webkit-early-warning-system commented Aug 23, 2022 •

edited

webkit-early-warning-system commented Aug 23, 2022 •

edited

webkit-early-warning-system commented Aug 24, 2022 •

edited

webkit-early-warning-system commented Aug 24, 2022 •

edited