Last Comment Bug 521969 - Cannot reference tooltip by a changed ID inside XBL
: Cannot reference tooltip by a changed ID inside XBL
Status: RESOLVED FIXED
[sg:dupe 522030]
: regression, verified1.9.0.15, verified1.9.0.16, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XBL (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 522030
Blocks: 489925
  Show dependency treegraph
 
Reported: 2009-10-13 03:43 PDT by Loune
Modified: 2012-05-07 10:29 PDT (History)
14 users (show)
dveditz: blocking1.9.0.15+
hskupin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta2-fixed
.4+
.4-fixed


Attachments
test.xul (366 bytes, application/vnd.mozilla.xul+xml)
2009-10-13 03:45 PDT, Loune
no flags Details
testxbl.xml (889 bytes, text/xml)
2009-10-13 03:47 PDT, Loune
no flags Details
test.css (50 bytes, text/css)
2009-10-13 03:47 PDT, Loune
no flags Details

Description Loune 2009-10-13 03:43:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.0.14) Gecko/2009082707 Firefox/3.0.14 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.1.4) Gecko/20091007 Firefox/3.5.4

If you changed the ID of a tooltip inside an XBL binding, you cannot refer to the new ID of the tooltip inside the XBL. (You can still however refer to it by the old ID). This was working as expected in 3.5.3.

Reproducible: Always

Steps to Reproduce:
1. See attachment example
Actual Results:  
Tool tip does not show up

Expected Results:  
Tool tip shows up
Comment 1 Loune 2009-10-13 03:45:37 PDT
Created attachment 406000 [details]
test.xul
Comment 2 Loune 2009-10-13 03:47:22 PDT
Created attachment 406001 [details]
testxbl.xml
Comment 3 Loune 2009-10-13 03:47:44 PDT
Created attachment 406002 [details]
test.css
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2009-10-13 04:01:01 PDT
Bz, do I remember right that you changed ID handling in anonymous content?
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 06:41:20 PDT
I changed the behavior of getElementById (to not return anonymous content).  I didn't change getAnonymousElementByAttribute.

Looking into this.
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 06:46:16 PDT
Oh, the tooltip thing.  I see...  nsXULTooltipListener::FindTooltip uses getElementById.  If it's supposed to work with XBL-generated content, that's wrong.  Sadly we had no tests for this.
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 06:49:19 PDT
The fact that this works without changing the id is incredibly scary, by the way.
Comment 8 neil@parkwaycc.co.uk 2009-10-13 06:54:39 PDT
In XBL (e.g. tabbrowser.xml) I think you're expected to use tooltip="_child".
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 06:57:18 PDT
OK, but are we fine with making the compat-breaking change of requiring that on our stable branches?
Comment 10 Neil Deakin 2009-10-13 07:06:43 PDT
id on xbl generated content isn't expected to be used, so I don't think there's anything to fix here.
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 07:20:28 PDT
And the reason it works to start with is this callstack:

#0  nsIdentifierMapEntry::AddIdContent (this=0x1f1bda68, aContent=0x849a00) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/base/src/nsDocument.cpp:420
420       NS_PRECONDITION(aContent, "Must have content");
(gdb) t
[Current thread is 1 (process 32789 local thread 0x2e03)]
(gdb) bt
#0  nsIdentifierMapEntry::AddIdContent (this=0x1f1bda68, aContent=0x849a00) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/base/src/nsDocument.cpp:420
#1  0x11f261e8 in nsDocument::UpdateIdTableEntry (this=0x1a97fa00, aContent=0x849a00) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/base/src/nsDocument.cpp:2333
#2  0x121a8ff8 in nsXULDocument::AddElementToDocumentPre (this=0x1a97fa00, aElement=0x849a00) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/xul/document/src/nsXULDocument.cpp:1704
#3  0x121aa81a in nsXULDocument::AddSubtreeToDocument (this=0x1a97fa00, aElement=0x849a00) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/xul/document/src/nsXULDocument.cpp:1786
#4  0x12167701 in nsXBLBinding::InstallAnonymousContent (this=0x20627340, aAnonParent=0x852820, aElement=0x2062fca0) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/content/xbl/src/nsXBLBinding.cpp:377
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 07:21:07 PDT
Well, we certainly need to fix the fact that it works to start with, before the attr change.
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-13 07:42:05 PDT
I filed bug 522030 on comment 11; unfortunately this needs to stay closed until that bug is opened up.

So this bug can stay on the desired behavior of nsXULTooltipListener::FindTooltip on the stable branches.  Should it find nodes with the relevant id in the same anonymous subtree if aTarget is anonymous?
Comment 14 Samuel Sidler (old account; do not CC) 2009-10-13 10:54:01 PDT
We're going to re-spin both Firefox 3.0.15 and 3.5.4 for this issue.
Comment 15 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-14 12:13:31 PDT
This is fixed on 1.9.2 by the checkin for bug 522030.

Per comment 8 and comment 10, sounds like this is invalid on trunk?
Comment 16 Samuel Sidler (old account; do not CC) 2009-10-15 13:33:23 PDT
Boris: This is fixed on 1.9.1 and 1.9.0 now, right?
Comment 17 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-15 13:55:14 PDT
Yes, it is.
Comment 18 Henrik Skupin (:whimboo) 2009-10-16 14:07:10 PDT
verified fixed on 1.9.1 and 1.9.2 with builds on all platforms:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.4) Gecko/20091016 Firefox/3.5.4 ID:20091016081620

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b2pre) Gecko/20091016 Namoroka/3.6b2pre ID:20091016042840

Boris, checking Minefield shows me the tooltip on all platforms. So I assume too that there is no problem on trunk (from my testing perspective at least).
Comment 19 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-16 14:32:38 PDT
The state on trunk is temporary; I still need to file the bug on backing out the fix for bug 522030 on trunk, relanding the fix for bug 489925 with some additional changes to better handle the situation of bug 522030, etc.  Once that's done this bug will reappear on trunk; the only question is whether it should be fixed (not that hard) or invalid/wontfix.
Comment 20 [On PTO until 6/29] 2009-10-16 16:10:01 PDT
I'm not sure what is going on in 1.9.0 for this bug.

If I run this on XP with 1.9.0.14 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.14) Gecko/2009082707 Firefox/3.0.14 (.NET CLR 3.5.30729)) using the attached testcase, run locally from my desktop, I get the "heya" tooltip. I see the same exact behavior in build 3 of 1.9.0.14 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.15) Gecko/2009101601 Firefox/3.0.15 (.NET CLR 3.5.30729)).

Was this bug actually ever explicitly seen in 1.9.0? Comment 0 implies it with the Build ID being present.
Comment 21 Henrik Skupin (:whimboo) 2009-10-16 16:18:01 PDT
Al, you mean build 3 of 3.0.15, right? You should check build 1 or 2 to see the difference. It was a regression from 3.0.14.
Comment 22 [On PTO until 6/29] 2009-10-16 16:30:08 PDT
Yes, I meant build 3 of 1.9.0.15 (see string following).

Comparing with Build 1, I see the failure there so this is verified fixed for 1.9.0.15.

Does this work in the same manner for bug 522030 (verify build 3 against build 1)?
Comment 23 Henrik Skupin (:whimboo) 2009-10-17 01:20:03 PDT
(In reply to comment #22)
> Does this work in the same manner for bug 522030 (verify build 3 against build
> 1)?

No, as given by Boris on bug 522030 we don't have a crash testcase yet.
Comment 24 [On PTO until 6/29] 2009-11-23 11:17:59 PST
Verified for 1.9.0.16 using attached testcase and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.16pre) Gecko/2009111921 GranParadiso/3.0.16pre (.NET CLR 3.5.30729).

Note You need to log in before you can comment on or make changes to this bug.