Issue 234100 in chromium: Tooltips in Shadow DOM don't work

[chro...@googlecode.com]

Status: Available
Owner: ----
CC: mor...@google.com
Labels: Type-Bug Pri-2 Cr-Blink-WebComponents OS-All WebKit-ID-108302

New issue 234100 by domi...@chromium.org: Tooltips in Shadow DOM don't work
http://code.google.com/p/chromium/issues/detail?id=234100

Migrated from WebKit Bugzilla:
https://bugs.webkit.org/show_bug.cgi?id=108302
Originally reported 2013-01-30 00:30 PST by Shinya Kawanaka
(shi...@chromium.org).


Description:
If nodes in ShadowDOM has title attribute, tooltip should be displayed
correctly.
However, the current behavior of tooltip is very weird. For example.

1) tooltip of host element is displayed
2) if another tooltip is shown, the tooltip for ShadowDOM is displayed on
the previous tooltip.

I believe hittest is not correctly implemented for ShadowDOM.
Also, this is very related to deprecatedShadowAncestorNode() bug (Bug 91821
[https://bugs.webkit.org/show_bug.cgi?id=91821]), as far as I see
EventHandler code.


Blocks:
[http://wkb.ug/91821] NEW - [Meta] Code calling shadowAncestorNode() does
not seem to consider nested ShadowDOM
[http://wkb.ug/97279] NEW - [Meta] Polish Shadow DOM until it shines

Attachments:
2013-01-30 18:14 PST: Patch
[https://bugs.webkit.org/attachment.cgi?id=185644&action=prettypatch]
2013-01-30 20:55 PST: Patch
[https://bugs.webkit.org/attachment.cgi?id=185670&action=prettypatch]



Comments:
================================

Comment #1
Posted on 2013-01-30 00:50:01 PST by Hajime Morrita (mor...@google.com)

Do you have any repro? It will help to let someone fix this.

================================

Comment #2
Posted on 2013-01-30 18:14:49 PST by Shinya Kawanaka (shi...@chromium.org)

Created an attachment (id=185644)
[https://bugs.webkit.org/attachment.cgi?id=185644] [details]
[https://bugs.webkit.org/attachment.cgi?id=185644&action=edit]
Patch

================================

Comment #3
Posted on 2013-01-30 18:17:12 PST by Shinya Kawanaka (shi...@chromium.org)

The patch contains repro.

I think we have two approaches for this issue.
1) If hit test result is in UA, we use host element instead.
2) If hit test result is in a shadow tree and there is no information for
tooltip, we use host element recursively.

I chose (1) approach for this patch, since this won't change behavior much.
But it might worth considering adopting (2) approach.

================================

Comment #4
Posted on 2013-01-30 18:41:15 PST by Hajime Morrita (mor...@google.com)

(From update of attachment 185644
[https://bugs.webkit.org/attachment.cgi?id=185644] [details]
[https://bugs.webkit.org/attachment.cgi?id=185644&action=edit])
View in context:
https://bugs.webkit.org/attachment.cgi?id=185644&action=review

> Source/WebCore/rendering/HitTestResult.h:141
> void setToNonShadowAncestor();

Can we just get rid of this one by replacing with new one?
Feels like having this is just wrong.

================================

Comment #5
Posted on 2013-01-30 18:52:49 PST by Shinya Kawanaka (shi...@chromium.org)

(In reply to comment #4 [https://bugs.webkit.org/show_bug.cgi?id=108302#c4])
> (From update of attachment 185644
> [https://bugs.webkit.org/attachment.cgi?id=185644] [details]
> [https://bugs.webkit.org/attachment.cgi?id=185644&action=edit] [details])
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=185644&action=review

> > Source/WebCore/rendering/HitTestResult.h:141
> > void setToNonShadowAncestor();

> Can we just get rid of this one by replacing with new one?
> Feels like having this is just wrong.

The other calling is in if(allowShadowContent), so I think we have to have
another method, which set nodes in document treescope.

================================

Comment #6
Posted on 2013-01-30 20:55:21 PST by Shinya Kawanaka (shi...@chromium.org)

Created an attachment (id=185670)
[https://bugs.webkit.org/attachment.cgi?id=185670] [details]
[https://bugs.webkit.org/attachment.cgi?id=185670&action=edit]
Patch

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[chro...@googlecode.com]

Updates:
Status: Started
Owner: morr...@chromium.org

Comment #1 on issue 234100 by mor...@google.com: Tooltips in Shadow DOM

don't work
http://code.google.com/p/chromium/issues/detail?id=234100

Should be a low hunging fruit - Shinya already had a patch. I can just
steal it (and spray some test.)

[chro...@googlecode.com]

Comment #3 on issue 234100 by bugdro...@chromium.org: Tooltips in Shadow
DOM don't work
http://code.google.com/p/chromium/issues/detail?id=234100#c3

------------------------------------------------------------------------
r150278 | mor...@chromium.org | 2013-05-14T02:01:50.303219Z

Changed paths:
A
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/dom/shadow/tooltips-in-shadow-expected.txt?r1=150278&r2=150277&pathrev=150278
M
http://src.chromium.org/viewvc/blink/trunk/Source/core/inspector/InspectorDOMAgent.cpp?r1=150278&r2=150277&pathrev=150278
M
http://src.chromium.org/viewvc/blink/trunk/Source/core/page/EventHandler.cpp?r1=150278&r2=150277&pathrev=150278
A
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/dom/shadow/tooltips-in-shadow.html?r1=150278&r2=150277&pathrev=150278
M
http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/HitTestResult.cpp?r1=150278&r2=150277&pathrev=150278
M
http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/HitTestResult.h?r1=150278&r2=150277&pathrev=150278

Made Blink tooltip ShadowDOM-aware

Before this change, tooltip lookup on EventHandler::mouseMoved() filtered
in-shadow nodes out. That results broken tooltip display of these in-shadow
elements.
Such filtering was originally introduced to hide UA shadows, but the
implementation
was a bit too aggressive that it filtered all shadow out.

This chagne add HitTestResult::setToShadowHostIfUserAgentShadowRoot().
This is a relaxed version of such filtering, which only hides nodes in UA
shadow.
Using this, tooltip is correctly located even if it is inside shadows.

BUG=234100
TEST=tooltips-in-shdaow.html

Review URL: https://chromiumcodereview.appspot.com/14677012
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Status: Fixed

Comment #4 on issue 234100 by mor...@google.com: Tooltips in Shadow DOM

don't work
http://code.google.com/p/chromium/issues/detail?id=234100

(No comment was entered for this change.)