Closed Bug 248862 Opened 18 years ago Closed 2 years ago

anonymous XBL content that are HTML elements does not show tooltips for the title attribute

Categories

(Core :: XBL, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: robert, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040510 Epiphany/1.2.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040510 Epiphany/1.2.4

Using a binding like this one

  <binding id="input">
    <content>
      <children includes="label"/>:
      <html:input type="text" title="This title is not displayed" anonid="input"/>
      <children/>
    </content>
  </binding>

works as designed, with the exception the the generated HTML input element does
not show the tooltip required by the title attribute. The same input element
added to the document directly (no XBL) shows the tooltip


Reproducible: Always
Steps to Reproduce:
1. add the binding to a element
2. go to the generated html input field
3. wait for the tooltip

Actual Results:  
the tooltip will not be showed

Expected Results:  
the tooltip must be showed like any non XBL generated HTML element
Attached file Testcase
Like this? This doesn't produce a tooltip when hovering over the link.
yes, nice testcase, I never thought on this method to add a binding only using
the XHTML file (binding on the same file) ;-)
bryner, does the tooltip listener need to get the original event target?
Keywords: testcase
Actually now that I think about it this bug is probably invalid; only XUL
supports tooltips, and the fact that tooltips appear for titled HTML elements in
web pages is simply some smart JavaScript plus XUL behind the scenes.
The title="" attribute for HTML elements should always generate a tooltip,
wherever that HTML element is.
Attached patch patchv1Splinter Review
This patch fixes the testcase and also when you use the context menu on the anonymous link element.
Attachment #207374 - Flags: review?(bryner)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 207374 [details] [diff] [review]
patchv1

Can you ask Neil and/or Boris to review this?  I'm not sure that exposing arbitrary anonymous content via document.popupNode is really a good idea.
Attachment #207374 - Flags: review?(bryner)
Hmm...  That worries me too.  I really wish we weren't storing this stuff on the document.  :(  As things stand, I'm not sure who's relying on document.popupNode being something in particular.

If nothing else, if we allow anon content in document.popupNode we should look over the security checks on that method and make sure they're up to snuff.
Flags: blocking1.9a1?
Comment on attachment 207374 [details] [diff] [review]
patchv1

>       if (eventTarget) {
>         nsCOMPtr<nsIContent> targetContent(do_QueryInterface(eventTarget));
>         mTargetNode = targetContent;
>       }
Heh, it seems to me that this is the equivalent of mTargetNode = do_QueryInterface(eventTarget); (since it should always be null prior to this point)

>+  nsEvent->GetOriginalTarget(getter_AddRefs(target));
>+  
>   nsCOMPtr<nsIDOMNode> targetNode = do_QueryInterface(target);
>+  
>+  nsCOMPtr<nsIContent> targetContent(do_QueryInterface(targetNode));
>+  
>+  if (targetContent->IsNativeAnonymous()) {
>+    for ( ; targetContent; targetContent = targetContent->GetParent()) 
>+    {
>+      if (!targetContent->IsNativeAnonymous()) {
>+        targetNode =  do_QueryInterface(targetContent);
>+        break;
>+      }
>+    }
>+  }
It seems to me that this could be written as follows:
nsEvent->GetOriginalTarget(getter_AddRefs(target));
nsCOMPtr<nsIContent> targetContent(do_QueryInterface(targetNode));
while (targetContent->IsNativeAnonymous())
  targetContent = targetContent->GetParent();
nsCOMPtr<nsIDOMNode> targetNode = do_QueryInterface(targetContent);

I forget the bug but I seem to remember someone requesting that originalTarget be retargetted for anonymous nodes which would in fact make the loop unnecessary.
Maybe nothing relies on popupNode, but tooltipNode is affected the same way, and that's used to position tooltips for HTML elements (from browser.js).

(In reply to comment #8)
> Hmm...  That worries me too.  I really wish we weren't storing this stuff on
> the document.  :(  As things stand, I'm not sure who's relying on
> document.popupNode being something in particular.
(In reply to comment #8)
>I really wish we weren't storing this stuff on the document.  :(
So document.originalPopupNode is out I guess...
>I'm not sure who's relying on document.popupNode being something in particular.
In our codebase, I can only find one use in mailWidgets.xml and that can probably be worked around using (display=?) extends="xul:button" - all the other elements with context menus already have an appropriate button or menu frame.

(In reply to comment #10)
>Maybe nothing relies on popupNode, but tooltipNode is affected the same way,
>and that's used to position tooltips for HTML elements (from browser.js).
Well we could easily fix that by positioning tooltips in screen coordinates.
In this case we'd _want_ to use the originalTarget for positioning, no?
(In reply to comment #9)
Yes, these suggestions seem to make all sense to me.

> I forget the bug but I seem to remember someone requesting that originalTarget
> be retargetted for anonymous nodes which would in fact make the loop
> unnecessary.
I think you mean bug 251197, comment 12, right?

(In reply to comment #7)
> (From update of attachment 207374 [details] [diff] [review] [edit])
> Can you ask Neil and/or Boris to review this?  I'm not sure that exposing
> arbitrary anonymous content via document.popupNode is really a good idea.
I don't really understand. With that targetContent->IsNativeAnonymous loop, you should never get native anonymous content, only xbl anonymous content, which isn't arbritrary anonymous content, is it? (but apparently I'm missing something here, please enlighten me)
Or should I use GetExplicitOriginalTarget? But afaics, that also retargets when it is xbl anonymous content.
Assignee: hyatt → general
Flags: blocking1.9a1? → blocking1.9-
Whiteboard: [wanted-1.9]
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Assignee: xbl → nobody
QA Contact: ian → xbl

XBL is now disabled in Firefox (Bug 1583314) and is in the process of being removed from Gecko (Bug 1566221), so closing bugs requesting changes to its implementation as wontfix.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.