467775 - XUL template creating nested elements instead of siblings

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: test XPI for Firefox 304/Shiretoko

A bug appeared on my mac between a build of BlueGriffon based off FF3.0.4 and a build based on mozilla-central's trunk. A sqlite database that seemed to be correctly created and used in a sqlite-based xul template with FF3.0.4 is horked with mozilla-central's trunk.

I have isolated the bug and created a small XPI for Firefox/Shiretoko. Step to reproduce the bug, please make sure you use two different profiles for FF304 and Shiretoko:

1. install the attached xpi
2. open Tools > Template bug
   (notice the file tplbug.sqlite that appeared in your profile)
4. the resulting dialog is perfectly ok on FF304 and horked on Shiretoko ; the
   former shows correctly the list of names in the database, the latter shows
   a very high corrupted list with only one visible entry.
5. now use Shiretoko on the profile you used in FF304 and open
   Tools >Templatebug ; notice this time the dialog is ok !
6. now use FF304 on the profile you used in Shiretoko and open the dialog,
   notice the dialog is horked
7. furthermore, the files tplbug.sqlite in the two profiles differ...

(warning, this extensions sets pref extension.tplbug.firstRun ; make sure
 to reset it if you need to clear your profile or recreate the sqlite database).

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Confirming on PC/WinXP too with FF304 and Shiretoko, fresh build from today.

Comment 2

[Shawn Wilsher :sdwilsh]

Just to clarify - how is the database horked - does it not open, or what?

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #2)
> Just to clarify - how is the database horked - does it not open, or what?

yes it does open. Sqlite Manager also sees/handles correctly the database.
Could be a sqlite-based template bug after all.

Comment 4

[Shawn Wilsher :sdwilsh]

Yeah, I'm not seeing anything here that indicates that it is a storage or sqlite bug

Comment 5

[Laurent Jouanneau]

I'm going to investigate on the template part..

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: screenshot of DOMI showing the issue

Ok, DOMI shows the problem... Listitems created by the template are created nested instead of being siblings !!! So the database does not seem to be guilty here. I'll update the title of the bug.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

flagging blocker191

Comment 8

[Laurent Jouanneau]

I made some tests with a simple database and a simple template (I'm not reusing the xpi of Daniel) and I have same issue.

I investigated and it seems that the bug appears since the landing of the patch for bug 450990. And it appears only with sqlite datasources (apparently, it is ok with rdf datasources).

So I think there is a bug in the query processor for storage, or perhaps in the content builder.

I take this bug.

Comment 9

[Shawn Wilsher :sdwilsh]

Could we get some tests too please so this doesn't break again in the future?

Comment 10

[Neil Deakin]

It looks offhand like http://mxr.mozilla.org/mozilla-central/source/content/xul/templates/src/nsXULContentBuilder.cpp#752 should be calling and checking MayProcessChildren like http://mxr.mozilla.org/mozilla-central/source/content/xul/templates/src/nsXULContentBuilder.cpp#1028 does.

Comment 11

[Neil Deakin]

Attachment: something like this

Laurent, this should fix this bug. Are you writing some tests for this?

Comment 12

[Laurent Jouanneau]

Yes Neil, I began to create tests and I will add them to those you already made for bug 378893. I'm going to test your patch, thanks Neil :-)

Comment 13

[Laurent Jouanneau]

Neil, this patch works for me :-) See my patch for bug 378893 to have tests.

Comment 14

[Neil Deakin]

Comment on attachment 351420 [details] [diff] [review]
something like this

This patch just ensures that GetMayProcessChildren, which always returns false for storage templates, is called in both cases.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 351420 [details] [diff] [review]
something like this

I can't really say I know this code well enough to review. Don't know who does though :(

Comment 16

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

FWIW, patch tested in BlueGriffon and works just fine.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 351420 [details] [diff] [review]
something like this


> nsXULContentBuilder::CreateContainerContents(nsIContent* aElement,
>                                              nsIXULTemplateResult* aResult,
>                                              PRBool aForceCreation,
>                                              PRBool aNotify,
>                                              PRBool aNotifyAtEnd)
> {
>     if (!aForceCreation && !IsOpen(aElement))
>         return NS_OK;
>+
>+    // don't generate children if recursion or child processing isn't allowed
>+    if (aResult != mRootResult) {
>+      if (mFlags & eDontRecurse)
>+        return NS_OK;
>+
>+      PRBool mayProcessChildren;
>+      nsresult rv = aResult->GetMayProcessChildren(&mayProcessChildren);
>+      if (NS_FAILED(rv) || !mayProcessChildren)
>+          return rv;
>+    }
Use 4 space indentation everywhere, not mixture of 2 and 4.