Bug 192640 – Make HTMLConverter work across shadow boundaries

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

We should make HTMLConverter produce a valid attributed string for a selection which crosses shadow boundaries.
Right now, this doesn't work and as a result, you can't copy & paste content across shadow boundaries from Safari to Notes, TextEdit, etc...

<rdar://problem/44968145>

[Ryosuke Niwa]

Created attachment 357208 [details]
Fixes the bug

[EWS Watchlist]

Comment on attachment 357208 [details]
Fixes the bug

Attachment 357208 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10378667

New failing tests:
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-2.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-1.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-2.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-4.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-3.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-1.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-5.html

[EWS Watchlist]

Created attachment 357213 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

Mac OS Sierra needs its own expected results :(

[Wenson Hsieh]

Comment on attachment 357208 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=357208&action=review

r=me, with test fixes!

> Source/WebCore/dom/Position.cpp:1606
> +    TreeScope* commonScope = commonTreeScope(a.containerNode(), b.containerNode());

Nit - auto*?

> Source/WebCore/editing/cocoa/HTMLConverter.mm:2321
> +                    Node* child = firstChildInComposedTreeIgnoringUserAgentShadow(node);

Nit - auto*?

> Source/WebCore/editing/cocoa/HTMLConverter.mm:2368
> +            Node* child = firstChildInComposedTreeIgnoringUserAgentShadow(element);

Nit - auto*?

> LayoutTests/editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-1.html:12
> +    dumpAttributedString(container, source, 0, shadowRoot.lastChild, shadowRoot.lastChild.data.indexOf('Kit'));

What is `source` here?

> LayoutTests/editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-2.html:15
> +    dumpAttributedString(container, source, 0, shadowRoot2.querySelector('u'), 1);

Here too?

[Ryosuke Niwa]

(In reply to Wenson Hsieh from comment #5)
> Comment on attachment 357208 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=357208&action=review
> 
> r=me, with test fixes!
> 
> > Source/WebCore/dom/Position.cpp:1606
> > +    TreeScope* commonScope = commonTreeScope(a.containerNode(), b.containerNode());
> 
> Nit - auto*?

Fixed.

> > Source/WebCore/editing/cocoa/HTMLConverter.mm:2321
> > +                    Node* child = firstChildInComposedTreeIgnoringUserAgentShadow(node);
> 
> Nit - auto*?

Fixed.

> > Source/WebCore/editing/cocoa/HTMLConverter.mm:2368
> > +            Node* child = firstChildInComposedTreeIgnoringUserAgentShadow(element);
> 
> Nit - auto*?

Fixed.

> > LayoutTests/editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-1.html:12
> > +    dumpAttributedString(container, source, 0, shadowRoot.lastChild, shadowRoot.lastChild.data.indexOf('Kit'));
> 
> What is `source` here?

Oops, supposed to be container. Fixed.

> > LayoutTests/editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-2.html:15
> > +    dumpAttributedString(container, source, 0, shadowRoot2.querySelector('u'), 1);
> 
> Here too?

Ditto.

[Ryosuke Niwa]

Created attachment 357255 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 357255 [details]
Patch for landing

Wait for EWS.

[EWS Watchlist]

Comment on attachment 357255 [details]
Patch for landing

Attachment 357255 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10388127

New failing tests:
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-1.html
editing/mac/attributed-string/attributed-string-across-shadow-boundaries-with-style-2.html

[EWS Watchlist]

Created attachment 357263 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

Created attachment 357269 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 357269 [details]
Patch for landing

Wait for EWS again

[Ryosuke Niwa]

Committed r239190: <https://trac.webkit.org/changeset/239190>