The CanHaveWhitespaceChildren optimization is not quite correct. |
|||||
Issue descriptionChrome Version: (copy from chrome://version) OS: (e.g. Win7, OSX 10.9.5, etc...) What steps will reproduce the problem? (1) Open attached test-case. What is the expected result? The two lines render in the same way. What happens instead? The line at the top doesn't show the whitespace in between "A" and "B", because of this line: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Text.cpp?l=286&rcl=d668cd5b0067c252e8c83d826a213d2d776b5c39. I disabled this optimization for display: contents, but as rune noticed it's broken more generally. I'm not sure I'll have the time to take this near-term, but the fix is presumably not too hard, so I'll try this evening to come up with something sound.
,
Jul 19 2017
,
Jul 19 2017
I think https://chromium-review.googlesource.com/c/575982/ is the right fix, let's see what CQ thinks.
,
Jul 20 2017
Removing Blink>Layout since in Style we try to have only one component per bug to make ownership clear.
,
Jul 20 2017
Assigning since you seem to be working on it :)
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3553bbf108ba4644dbca90e0e39f805fcda5aeb commit f3553bbf108ba4644dbca90e0e39f805fcda5aeb Author: Emilio Cobos Álvarez <ecobos@igalia.com> Date: Tue Aug 01 13:40:58 2017 Account for sibling text nodes in CanHaveWhitespaceChildren. Not doing it results in incorrect behavior when there are multiple adjacent text nodes, either script-generated ones or because of display: contents. BUG= 746437 , 657748 Change-Id: If5aa9209d16d5532629a0f0e2f1730d18a193ebf Reviewed-on: https://chromium-review.googlesource.com/575982 Reviewed-by: Rune Lillesveen <rune@opera.com> Commit-Queue: Emilio Cobos Álvarez <ecobos@igalia.com> Cr-Commit-Position: refs/heads/master@{#490971} [modify] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/LayoutTests/fast/css/whitespace-between-text-flex-expected.html [add] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/LayoutTests/fast/css/whitespace-between-text-flex-preserve-expected.html [add] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/LayoutTests/fast/css/whitespace-between-text-flex-preserve.html [add] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/LayoutTests/fast/css/whitespace-between-text-flex.html [modify] https://crrev.com/f3553bbf108ba4644dbca90e0e39f805fcda5aeb/third_party/WebKit/Source/core/dom/Text.cpp
,
Aug 7 2017
This can be marked as fixed now?
,
Aug 7 2017
Yes, good call :) |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by eco...@igalia.com
, Jul 19 2017