Summary: | [iOS WK2] Can't place caret or select in content that overflows a contenteditable element | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | HTML Editing | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | megan_gardner, sam, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2019-07-11 17:51:13 PDT
Created attachment 373985 [details]
Patch
Created attachment 373986 [details]
Patch
> Test suite failed
> Failed
> TestWebKitAPI.KeyboardInputTests.SelectionClipRectsWhenPresentingInputView
>
> /Volumes/Data/worker/iOS-12-Simulator-Build-EWS/build/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:524
> Expected equality of these values:
> 10
> selectionClipRect.origin.x
> Which is: 11
It looks like this can just be rebaselined.
Created attachment 374028 [details]
Patch
Comment on attachment 374028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374028&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1610 > + boundingBox.setWidth(boundingBox.width() - style.borderLeftWidth() - style.borderRightWidth()); > + boundingBox.setHeight(boundingBox.height() - style.borderBottomWidth() - style.borderTopWidth()); Nit - could consider using contract instead. > LayoutTests/editing/caret/ios/caret-in-overflow-area.html:47 > + window.addEventListener('load', async () => { > + runTest(); > + }, false); Nit - addEventListener(‘load’, runTest);? > LayoutTests/editing/selection/ios/place-selection-in-overflow-area.html:53 > + window.addEventListener('load', async () => { > + runTest(); > + }, false); Ditto. > LayoutTests/editing/selection/ios/selection-extends-into-overflow-area.html:61 > + window.addEventListener('load', async () => { > + runTest(); > + }, false); Ditto. Comment on attachment 374028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374028&action=review > Source/WebKit/WebProcess/WebPage/WebPage.h:1212 > +#if PLATFORM(IOS_FAMILY) > + // This excludes layout overflow, includes borders. > + static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&); > + // These include layout overflow for overflow:visible elements, but exclude borders. > + static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&); > + static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&); > +#endif // PLATFORM(IOS_FAMILY) Do these really need to be iOS specific? They seem pretty general. We really should be aiming to have less platform specific code like this. (In reply to Sam Weinig from comment #8) > Comment on attachment 374028 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=374028&action=review > > > Source/WebKit/WebProcess/WebPage/WebPage.h:1212 > > +#if PLATFORM(IOS_FAMILY) > > + // This excludes layout overflow, includes borders. > > + static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&); > > + // These include layout overflow for overflow:visible elements, but exclude borders. > > + static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&); > > + static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&); > > +#endif // PLATFORM(IOS_FAMILY) > > Do these really need to be iOS specific? They seem pretty general. We really > should be aiming to have less platform specific code like this. Agreed. I started to clean things up and it got messy; it's unclear if all the callers want to include borders or not, and which want the interaction rect vs. the content box rect. (In reply to Simon Fraser (smfr) from comment #9) > (In reply to Sam Weinig from comment #8) > > Comment on attachment 374028 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=374028&action=review > > > > > Source/WebKit/WebProcess/WebPage/WebPage.h:1212 > > > +#if PLATFORM(IOS_FAMILY) > > > + // This excludes layout overflow, includes borders. > > > + static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&); > > > + // These include layout overflow for overflow:visible elements, but exclude borders. > > > + static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&); > > > + static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&); > > > +#endif // PLATFORM(IOS_FAMILY) > > > > Do these really need to be iOS specific? They seem pretty general. We really > > should be aiming to have less platform specific code like this. > > Agreed. I started to clean things up and it got messy; it's unclear if all > the callers want to include borders or not, and which want the interaction > rect vs. the content box rect. Will you finish :) ? Since these are static, and don't rely on the Page at all, it might also be better to move them to their own ElementGeometry.h/cpp (or whatever) file. And/Or move them to WebCore. Not right now. (In reply to Simon Fraser (smfr) from comment #11) > Not right now. Bummer. |