Bug 272273
| Summary: | Remove unused API function `hasNonEmptyBox` added in 49632@main for Chromium port | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Forms | Assignee: | 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
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 | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Ryosuke Niwa
HTMLAnchorElement::isKeyboardFocusable seems to be using that function?
Ahmad Saleem
(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
(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
(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
<rdar://problem/126395865>
Ryosuke Niwa
Pull request: https://github.com/WebKit/WebKit/pull/35092
EWS
Committed 285065@main (d2369dc2b1b7): <https://commits.webkit.org/285065@main>
Reviewed commits have been landed. Closing PR #35092 and removing active labels.