Bug 272273

Summary: Remove unused API function `hasNonEmptyBox` added in 49632@main for Chromium port
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: FormsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, rniwa, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   

Ahmad Saleem
Reported 2024-04-06 00:01:13 PDT
Hi Team, While doing 275976@main, I tried to remove but got comment that it should be done later. I tried to see if it leads to any test failure - I was under the impressions that it would progress `bug 31496` based on comment https://bugs.webkit.org/show_bug.cgi?id=31496#c7 , although it started to work without removing this code in STP191 so my fix to deal with edge case `275976@main` fixed it. NOTE - I have tried to come up with test by adding `display: contents` or `display: content` around <a> to see if it leads without renderer to cause test failure but it didn't. `Purpose: 1. WebElement::isVisible — tells whether a DOM element is visible (defined as 'has a layout with a non-empty bounding box').` [We don't have such use case and Blink also removed this code as well]. Now back to main topic why this is unused. It was added by bug 37625 and it was for Chromium port originally in Node.cpp|h but later it was moved to HTMLAnchorElement.cpp. https://github.com/WebKit/WebKit/commit/558a9e8febda68a0a49ccc98a21287660b38c1bc NOTE - in previous attempt in 275976@main, it didn't lead to any test failure as well, so I think we can delete it without any repercussions. Just raising to get input so I can do without any test to something since we never had used it. Thanks!
Attachments
Ryosuke Niwa
Comment 1 2024-04-06 23:03:08 PDT
HTMLAnchorElement::isKeyboardFocusable seems to be using that function?
Ahmad Saleem
Comment 2 2024-04-09 17:28:29 PDT
(In reply to Ryosuke Niwa from comment #1) > HTMLAnchorElement::isKeyboardFocusable seems to be using that function? It is relic from Chromium use case (https://bugs.webkit.org/show_bug.cgi?id=31496#c7), which we kept and removing it - does not lead to any test regressions.
Ryosuke Niwa
Comment 3 2024-04-09 17:43:25 PDT
(In reply to Ahmad Saleem from comment #2) > (In reply to Ryosuke Niwa from comment #1) > > HTMLAnchorElement::isKeyboardFocusable seems to be using that function? > > It is relic from Chromium use case > (https://bugs.webkit.org/show_bug.cgi?id=31496#c7), which we kept and > removing it - does not lead to any test regressions. That doesn't mean we can just remove the code.
Ahmad Saleem
Comment 4 2024-04-09 22:01:01 PDT
(In reply to Ryosuke Niwa from comment #3) > (In reply to Ahmad Saleem from comment #2) > > (In reply to Ryosuke Niwa from comment #1) > > > HTMLAnchorElement::isKeyboardFocusable seems to be using that function? > > > > It is relic from Chromium use case > > (https://bugs.webkit.org/show_bug.cgi?id=31496#c7), which we kept and > > removing it - does not lead to any test regressions. > > That doesn't mean we can just remove the code. I shared wrong link - https://bugs.webkit.org/show_bug.cgi?id=37625 , this added it.
Radar WebKit Bug Importer
Comment 5 2024-04-13 00:02:13 PDT
Ryosuke Niwa
Comment 6 2024-10-11 22:01:19 PDT
EWS
Comment 7 2024-10-12 00:39:07 PDT
Committed 285065@main (d2369dc2b1b7): <https://commits.webkit.org/285065@main> Reviewed commits have been landed. Closing PR #35092 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.