The spreadsheet title in a Quip spreadsheet is an editable inline element. When it has no content, which can happen if you delete it, then it has zero width and in this state it is not found by -_requestTextInputContextsInRect.
<rdar://problem/59355847>
Created attachment 402674 [details] Patch
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review > Source/WebCore/ChangeLog:12 > + and painting them. It's complicated to patch up all this up with a 100% correct this up => this
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review > Source/WebCore/ChangeLog:17 > + (They can only be found when focused using a keyboard or random tapping/clicking). random tapping/clicking => tapping/clicking on them
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review > Source/WebCore/ChangeLog:17 > + solution without forcing more line box creation. It's clear to me that the customer > + experience will be significantly better if the focused editable no line box case > + is handled, but it's unclear whether that can be topped by patching everything to > + track all editable inlines without a line box because they are not discoverable. > + (They can only be found when focused using a keyboard or random tapping/clicking). Maybe you don't need all these words. > Source/WebCore/platform/graphics/FloatRect.cpp:57 > +bool FloatRect::intersectsEvenIfEmpty(const FloatRect& other) const This is ambiguous about which rect might be empty. Is it |this| or other or both?
It's clear to me that the customer experience will be significantly better if the focused editable no line box case is handled, but it's unclear whether that can be topped by patching everything to track all editable inlines without a line box because they are not discoverable. (They can only be found when focused using a keyboard or tapping/clicking on them). So, I chose to fix this bug by adding a special case in the code.
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review >>> Source/WebCore/ChangeLog:17 >>> + (They can only be found when focused using a keyboard or random tapping/clicking). >> >> random tapping/clicking => tapping/clicking on them > > Maybe you don't need all these words. Copied a bunch of ^^^ words into a bugzilla comment to capture them...I'll remove them from the change log >> Source/WebCore/platform/graphics/FloatRect.cpp:57 >> +bool FloatRect::intersectsEvenIfEmpty(const FloatRect& other) const > > This is ambiguous about which rect might be empty. Is it |this| or other or both? Both could be empty. I just was following the naming convention of uniteIfEmpty(). I can't think of a better name, right now.
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review >>> Source/WebCore/platform/graphics/FloatRect.cpp:57 >>> +bool FloatRect::intersectsEvenIfEmpty(const FloatRect& other) const >> >> This is ambiguous about which rect might be empty. Is it |this| or other or both? > > Both could be empty. I just was following the naming convention of uniteIfEmpty(). I can't think of a better name, right now. Renaming inclusiveIntersection() on your private chat suggestion.
Thanks Simon!
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review >>>> Source/WebCore/platform/graphics/FloatRect.cpp:57 >>>> +bool FloatRect::intersectsEvenIfEmpty(const FloatRect& other) const >>> >>> This is ambiguous about which rect might be empty. Is it |this| or other or both? >> >> Both could be empty. I just was following the naming convention of uniteIfEmpty(). I can't think of a better name, right now. > > Renaming inclusiveIntersection() on your private chat suggestion. Actually renaming to inclusivelyIntersects
Created attachment 402815 [details] To land
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review Heh, just realized I never pushed publish. My comment is consistent with what the two of you did! > Source/WebCore/platform/graphics/FloatRect.h:165 > + WEBCORE_EXPORT bool intersectsEvenIfEmpty(const FloatRect&) const; Would be nice to find a more graceful name for this operation. I guess this is like uniteEvenIfEmpty. There have to be some better terms of art for this sort of thing since they come up in every single graphics library!
Created attachment 402816 [details] To land
Created attachment 402818 [details] To land
Committed r263531: <https://trac.webkit.org/changeset/263531>
Comment on attachment 402674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402674&action=review >> Source/WebCore/platform/graphics/FloatRect.h:165 >> + WEBCORE_EXPORT bool intersectsEvenIfEmpty(const FloatRect&) const; > > Would be nice to find a more graceful name for this operation. I guess this is like uniteEvenIfEmpty. There have to be some better terms of art for this sort of thing since they come up in every single graphics library! I went with inclusivelyIntersects in the latest patch, just landed.