745712 - FillInHTMLTooltip should not use title attributes from XUL ascendants

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

When hovering the <div> in this example:

<xul:window title="foobar">
  <div></div>
</xul:window>

FillInHTMLTooltip will not find a "title" attribute on the div itself and thus walk up its parents until one of them has a title set. We need to ignore XUL parents when doing this.

Comment 1

[Dão Gottwald [:dao]]

Comment on attachment 615302 [details] [diff] [review]
patch v1

>   // Don't show the tooltip if the tooltip node is a XUL element, a document or is disconnected.
>-  if (tipElement.namespaceURI == "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" ||
>+  if (tipElement.namespaceURI == XULNS ||

Does this check still make sense?

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Dão Gottwald [:dao] from comment #1)
> Does this check still make sense?

If we remove this check then we'd see a tooltip in case we hover the <xul:vbox> in this example:

<div title="foobar">
  <xul:vbox></xul:vbox>
</div>

So I think it make sense to remove it. What do you think?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Removed the first check.

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 615308 [details] [diff] [review]
patch v2

>   // Don't show the tooltip if the tooltip node is a XUL element, a document or is disconnected.
>-  if (tipElement.namespaceURI == "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" ||
>-      !tipElement.ownerDocument ||
>+  if (!tipElement.ownerDocument ||

// Don't show the tooltip if the tooltip node is a document or disconnected.

>+  let XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";

const

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/1e7f193ff1a7

Comment 6

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/1e7f193ff1a7

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 615308 [details] [diff] [review]
patch v2

[Approval Request Comment]
Regression caused by (bug #): no regression
Testing completed (on m-c, etc.): landed on m-c some days ago
Risk to taking this patch (and alternatives if risky): very small patch and risk
String changes made by this patch: none

We need this as a prerequisite for bug 736476.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/621430357bac

Comment 9

[Ioana (away)]

Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0
BuildID: 20120528154913

1.
<xul:window title="foobar">
  <div>divvvvv</div>
</xul:window>

Results: "foobar" is displayed when hovering over the div.

2.
<xul:window title="foobar">
  <div title="title11">divvvvv</div>
</xul:window>

Results: "title11" is displayed when hovering over the div.

3.
<div title="foobar">
  <xul:vbox>vboxxxxx</xul:vbox>
</div>

Results: "foobar" is displayed when hovering over the vbox.

Tim, are these results as expected? At point 1, shouldn't there be not title displayed?

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ioana Budnar [QA] from comment #9)
> Tim, are these results as expected? At point 1, shouldn't there be not title
> displayed?

(1) is the same structure as we have it on the newtab page and it shouldn't show the title. How exactly did you test these code snippets?

Comment 11

[Ioana (away)]

(In reply to Tim Taubert [:ttaubert] from comment #10)
> (In reply to Ioana Budnar [QA] from comment #9)
> > Tim, are these results as expected? At point 1, shouldn't there be not title
> > displayed?
> 
> (1) is the same structure as we have it on the newtab page and it shouldn't
> show the title. How exactly did you test these code snippets?

I added them in a html file that I loaded in Firefox. Is there another way you need me to test them?

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ioana Budnar [QA] from comment #11)
> I added them in a html file that I loaded in Firefox. Is there another way
> you need me to test them?

Since remote XUL got disabled I think this is not seen as a "real" XUL element but rather a HTML element with a custom namespace. You'd probably need to modify a XUL file shipped with Firefox or included via some add-on.

Comment 13

[juan becerra [:juanb]]

I tried using the "remote xul manager" add-on and some examples for xul windows, but I wasn't able to show the tooltip for the divs in the first place, but if you have other suggestions, I could take another look at verifying this.

Comment 14

[u279076]

QA attempts to reproduce this so we can accurately verify the fix have been unsuccessful. Untracking for QA verification. If there is someone who can reproduce this bug, please assist us in the verification; we need this tested in Firefox 13. If you can provide QA a test to reproduce, please set the whiteboard tag back to [qa+].

Thank you.