Changeset 248368 in webkit
- Timestamp:
- Aug 7, 2019 9:23:52 AM (5 years ago)
- Location:
- trunk
- Files:
-
- 2 added
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r248338 r248368 1 2019-08-07 Wenson Hsieh <wenson_hsieh@apple.com> 2 3 Extra space inserted at start of line when inserting a newline in Mail compose 4 https://bugs.webkit.org/show_bug.cgi?id=200490 5 <rdar://problem/53501354> 6 7 Reviewed by Antti Koivisto. 8 9 Add a new test to verify that inserting a newline in the middle of text in a document with designMode "on" 10 doesn't insert an extra space at the beginning of the newly inserted line. 11 12 * editing/inserting/insert-paragraph-in-designmode-document-expected.txt: Added. 13 * editing/inserting/insert-paragraph-in-designmode-document.html: Added. 14 1 15 2019-08-06 Ryosuke Niwa <rniwa@webkit.org> 2 16 -
trunk/Source/WebCore/ChangeLog
r248366 r248368 1 2019-08-07 Wenson Hsieh <wenson_hsieh@apple.com> 2 3 Extra space inserted at start of line when inserting a newline in Mail compose 4 https://bugs.webkit.org/show_bug.cgi?id=200490 5 <rdar://problem/53501354> 6 7 Reviewed by Antti Koivisto. 8 9 This started happening after r244494, which deferred editor state computation until the next layer tree flush 10 when changing selection. After inserting a paragraph, the act of computing an editor state ensured that the text 11 node containing the caret drops out of simple line layout, while grabbing the characters near the selection 12 (i.e., calling charactersAroundPosition). This meant that when we subsequently ask positionAfterSplit whether it 13 isRenderedCharacter() at the end of the command, we are guaranteed to have line boxes, so we get a meaningful 14 answer and avoid inserting an extra non-breaking space. 15 16 However, after r244494, we defer the editor state computation until the end of the edit command; this means that 17 we may not have line boxes for positionAfterSplit's text node renderer, due to remaining in simple line layout. 18 In turn, this means that we end up hitting the assertion in containsRenderedCharacterOffset in debug builds; on 19 release builds, we simply return false from containsRenderedCharacterOffset, which causes us to insert an extra 20 space. 21 22 To fix this, we educate RenderText::containsRenderedCharacterOffset about simple line layout. 23 24 Test: editing/inserting/insert-paragraph-in-designmode-document.html 25 26 * rendering/RenderText.cpp: 27 (WebCore::RenderText::containsRenderedCharacterOffset const): 28 (WebCore::RenderText::containsCaretOffset const): 29 30 Changed to use SimpleLineLayout::containsOffset. 31 32 * rendering/SimpleLineLayoutFunctions.h: 33 (WebCore::SimpleLineLayout::containsOffset): 34 35 I first contrasted the behavior of RenderTextLineBoxes::containsOffset in the cases where the OffsetType is 36 CaretOffset or CharacterOffset, and found that the only interesting differences were: 37 38 1. The caret offset type case has special handling for line breaks. 39 2. Both offset types have handling for reversed text. 40 3. The end offset of a line box contains a caret offset, but not a character offset. 41 42 For the purposes of OffsetType CharacterOffset, (1) is irrelevant; furthermore, (2) is already not handled by 43 logic in containsCaretOffset(). Thus, the only major difference in the CharacterOffset case should be (3), which 44 we handle by only allowing the case where the given offset is equal to the very end of a text run for caret 45 offsets, and not character offsets. 46 47 (WebCore::SimpleLineLayout::containsCaretOffset): Deleted. 48 49 Renamed to just containsOffset. 50 1 51 2019-08-07 Youenn Fablet <youenn@apple.com> 2 52 -
trunk/Source/WebCore/rendering/RenderText.cpp
r246951 r248368 1490 1490 bool RenderText::containsRenderedCharacterOffset(unsigned offset) const 1491 1491 { 1492 ASSERT(!simpleLineLayout()); 1492 if (auto* layout = simpleLineLayout()) 1493 return SimpleLineLayout::containsOffset(*this, *layout, offset, SimpleLineLayout::OffsetType::CharacterOffset); 1493 1494 return m_lineBoxes.containsOffset(*this, offset, RenderTextLineBoxes::CharacterOffset); 1494 1495 } … … 1497 1498 { 1498 1499 if (auto* layout = simpleLineLayout()) 1499 return SimpleLineLayout::contains CaretOffset(*this, *layout, offset);1500 return SimpleLineLayout::containsOffset(*this, *layout, offset, SimpleLineLayout::OffsetType::CaretOffset); 1500 1501 return m_lineBoxes.containsOffset(*this, offset, RenderTextLineBoxes::CaretOffset); 1501 1502 } -
trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.h
r230914 r248368 51 51 52 52 bool isTextRendered(const RenderText&, const Layout&); 53 bool containsCaretOffset(const RenderObject&, const Layout&, unsigned); 53 enum class OffsetType { CaretOffset, CharacterOffset }; 54 bool containsOffset(const RenderText&, const Layout&, unsigned, OffsetType); 54 55 unsigned findCaretMinimumOffset(const RenderObject&, const Layout&); 55 56 unsigned findCaretMaximumOffset(const RenderObject&, const Layout&); … … 117 118 } 118 119 119 inline bool contains CaretOffset(const RenderText&, const Layout& layout, unsigned offset)120 inline bool containsOffset(const RenderText&, const Layout& layout, unsigned offset, OffsetType offsetType) 120 121 { 121 122 for (unsigned i = 0; i < layout.runCount(); ++i) { … … 123 124 if (offset < run.start) 124 125 return false; 125 if (offset < = run.end)126 if (offset < run.end || (offsetType == OffsetType::CaretOffset && offset == run.end)) 126 127 return true; 127 128 }
Note: See TracChangeset
for help on using the changeset viewer.