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

Remove non-standard width() and height() functions of HTMLDocument #26082

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Mar 18, 2024

9e4d690

Remove non-standard `width()` and `height()` functions of HTMLDocument

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

Reviewed by Ryosuke Niwa and Michael Catanzaro.

This patch is to remove remaining code for dropped `HTMLDocument.width / height`
attributes, which were dropped in 171244@main (via IDL changes) and later
removed in 179619@main (both in 2016 - via removal from IDL).

* Source/WebCore/html/HTMLDocument.cpp:
(HTMLDocument::width): Deleted
(HTMLDocument::height): Deleted
* Source/WebCore/html/HTMLDocument.h:
* Source/WebKitLegacy/mac/DOM/DOMHTMLDocument.mm:
(width):
(height):
* Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMHTMLDocument.cpp:
(webkit_dom_html_document_get_width):
(webkit_dom_html_document_get_height):

Canonical link: https://commits.webkit.org/276488@main

450f296

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ api-gtk
  πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@Ahmad-S792 Ahmad-S792 added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Mar 18, 2024
@Ahmad-S792 Ahmad-S792 self-assigned this Mar 18, 2024
@Ahmad-S792 Ahmad-S792 changed the title Remove unused width() and height()functions of HTMLDocument (from WebKit and WebKitLegacy) Remove unused width() and height() functions of HTMLDocument (from WebKit and WebKitLegacy) Mar 18, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 18, 2024
@Ahmad-S792 Ahmad-S792 force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch from bc9b1fc to 2a7c420 Compare March 19, 2024 00:32
@Ahmad-S792 Ahmad-S792 requested a review from a team as a code owner March 19, 2024 00:32
@Ahmad-S792 Ahmad-S792 force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch from 2a7c420 to 5cb57af Compare March 19, 2024 01:02
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Mar 19, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 19, 2024
Copy link
Contributor

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I would remove " (from WebKit and WebKitLegacy)" as it's inaccurate. These are still exposed in WebKitLegacy.

@Ahmad-S792 Ahmad-S792 force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch from 5cb57af to 1f1978a Compare March 19, 2024 11:30
@Ahmad-S792 Ahmad-S792 changed the title Remove unused width() and height() functions of HTMLDocument (from WebKit and WebKitLegacy) Remove non-standard width() and height() functions of HTMLDocument Mar 19, 2024
@annevk
Copy link
Contributor

annevk commented Mar 19, 2024

Thanks! This looks good to me, but I'd prefer @rniwa / @cdumez to sign off and someone from the Linux ports for the API changes.

@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Mar 19, 2024
@Ahmad-S792 Ahmad-S792 force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch 2 times, most recently from 798f2db to f35e335 Compare March 19, 2024 22:41
@Ahmad-S792 Ahmad-S792 force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch from f35e335 to 450f296 Compare March 20, 2024 22:15
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 21, 2024
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Mar 21, 2024
Copy link
Contributor

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

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

The DOM API has been deprecated since WebKitGTK version 2.22.0 (September 2018, four and a half years ago), and has never been part of WPE. Building WebKitGTK against GTK4 disables the whole DOM API. At this point we would expect everybody to have moved on; we only keep the library and its symbols around to prevent old applications that haven't been updated from segfaulting at load time.

I think this change is okay for the GTK port, if @carlosgcampos agrees let's merge it.

Copy link
Contributor

@mcatanzaro mcatanzaro left a comment

Choose a reason for hiding this comment

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

If Apple is OK with the API changes in DOMHTMLDocument.mm, then this is good.

@carlosgcampos
Copy link
Contributor

LGTM

@Ahmad-S792 Ahmad-S792 added the merge-queue Applied to send a pull request to merge-queue label Mar 21, 2024
@webkit-commit-queue 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 Mar 21, 2024
@Ahmad-S792 Ahmad-S792 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 Mar 21, 2024
https://bugs.webkit.org/show_bug.cgi?id=271190

Reviewed by Ryosuke Niwa and Michael Catanzaro.

This patch is to remove remaining code for dropped `HTMLDocument.width / height`
attributes, which were dropped in 171244@main (via IDL changes) and later
removed in 179619@main (both in 2016 - via removal from IDL).

* Source/WebCore/html/HTMLDocument.cpp:
(HTMLDocument::width): Deleted
(HTMLDocument::height): Deleted
* Source/WebCore/html/HTMLDocument.h:
* Source/WebKitLegacy/mac/DOM/DOMHTMLDocument.mm:
(width):
(height):
* Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMHTMLDocument.cpp:
(webkit_dom_html_document_get_width):
(webkit_dom_html_document_get_height):

Canonical link: https://commits.webkit.org/276488@main
@webkit-commit-queue webkit-commit-queue force-pushed the fix271190-remove-code-for-HTMLDocument-width-height branch from 450f296 to 9e4d690 Compare March 21, 2024 20:12
@webkit-commit-queue
Copy link
Collaborator

Committed 276488@main (9e4d690): https://commits.webkit.org/276488@main

Reviewed commits have been landed. Closing PR #26082 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 21, 2024
@webkit-commit-queue webkit-commit-queue merged commit 9e4d690 into WebKit:main Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
9 participants