Bug 273219
Summary: | Stop TextIterator from checking margin when adding double newlines after <p> | ||
---|---|---|---|
Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED DUPLICATE | ||
Severity: | Normal | CC: | annevk, rniwa |
Priority: | P2 | ||
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
Ahmad Saleem
Hi Team,
While looking into Blink commits, I noticed the test expectation changes between our current vs Blink and noticed that we don't have this merged / aligned:
Blink Commit: https://chromium.googlesource.com/chromium/src.git/+/8ff781cd5c1aabca068247de9a3f143645e80422
I took following test as example: fast/css-generated-content/hover-inline-expected.txt (See from source.chromium.org vs GitHub webkit repo) - Chrome / Blink adds additional space.
WebKit Source: https://github.com/WebKit/WebKit/blob/983b6c8a16f0fbe166336a0c9653cd8eca72f639/Source/WebCore/editing/TextIterator.cpp#L929
___
In above, if I do following:
Current:
RefPtr element = dynamicDowncast<HTMLElement>(node);
if (!element || (!hasHeaderTag(*element) && !is<HTMLParagraphElement>(*element)))
return false;
to:
RefPtr element = dynamicDowncast<HTMLElement>(node);
if (is<HTMLParagraphElement>(*element))
return true;
if (!element || !hasHeaderTag(*element))
return false;
___
In debug build, it leads to following 'assert' while running WPT test (e.g., https://wpt.fyi/results/html/dom/elements/the-innertext-and-outertext-properties/getter.html?label=master&label=experimental&aligned&q=innertext)
ASSERTION FAILED: m_ptr
/Users/ahmadsaleem/Documents/GitHub/WebKit/WebKitBuild/Debug/usr/local/include/wtf/RefPtr.h(68) : T &WTF::RefPtr<WebCore::HTMLElement>::operator*() const [T = WebCore::HTMLElement, _PtrTraits = WTF::RawPtrTraits<WebCore::HTMLElement>, _RefDerefTraits = WTF::DefaultRefDerefTraits<WebCore::HTMLElement>]
2024-04-24 19:45:05.336 MiniBrowser[43249:479598] WebContent process crashed; reloading
___
Just wanted to raise so we can track and fix it.
Thanks!
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Ahmad Saleem
We can just change this whole function to follow like this:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/editing/iterators/text_iterator.cc;l=702?q=the-innertext-idl-attribute
and it will or might progress few WPT tests in: https://wpt.fyi/results/html/dom/elements/the-innertext-and-outertext-properties/getter.html?label=master&label=experimental&aligned&q=innertext
Ahmad Saleem
This progresses six tests without leading to crashes:
static bool shouldEmitExtraNewlineForNode(Node& node)
{
// https://html.spec.whatwg.org/multipage/dom.html#the-innertext-idl-attribute
// // Append two required linebreaks after a <p> element.
CheckedPtr renderBox = dynamicDowncast<RenderBox>(node.renderer());
if (!renderBox || !renderBox->height())
return false;
return node.hasTagName(pTag);
}
Ahmad Saleem
*** This bug has been marked as a duplicate of bug 176776 ***