83651 - get import precedence right

Status: VERIFIED FIXED

(Core :: XSLT, defect, P3)

Description

[Axel Hecht]

import precedence rulez
xsl:template
xsl:strip-space, xsl:preserve-space
xsl:namespace-alias
xsl:attribute-set
xsl:variable, xsl:param (just top level vars, if I read that right)
xsl:output

xsl:decimal-format, xsl:key do not, if I got the spec right

We could have a list of txImportFrames (or so), to store this information.

Axel

Comment 1

[Axel Hecht]

setting milestone on the bugs that nisheeth wanted. Setting Priority up for
landing indication

Axel

Comment 2

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

pushing

Comment 3

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

Attachment: Precedence for attribsets, templates and whitespacefiltering. Ver 1

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 50836 [details] [diff] [review]
Precedence for attribsets, templates and whitespacefiltering. Ver 1

r=peterv, though I'm not confident I did a very thorough review. I want Pike to r it too.

Small nits:
-I prefer the { on a newline after a function definition
-No newline after a while () {
-Prefer mFoo for members and aFoo for parameters

The files you touched don't follow a particular style, so you could ignore those and leave them for the cleanup bug.

Comment 5

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

I got the following comments from Pike:

ProcessorState::addTemplate
- name should be const, too.
- match should be below the name check
- get a const string for mode, too.

ProcessorState::findTemplate
- how about changing the signature from String* aMode to const String& aMode?
- don't cut the lines
+                String priorityAttr = templ->
+                    mTemplate->getAttribute(PRIORITY_ATTR);
this way, 
+                String priorityAttr = 
+                    templ->mTemplate->getAttribute(PRIORITY_ATTR);
is better. The second instance, too. I don't like long lines, but this
is worse.

ProcessorState::getFoo:
+    while (frame = (ImportFrame*)frameIter.previous()) {
should be
+    while ((frame = (ImportFrame*)frameIter.previous())) {
IIRC

a macro for 
(ProcessorState::ImportFrame*)importFrame->current())
in XSLTProcessor?

fix the String() to NULL_STRING in XSLTProcessor

Comment 6

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

Attachment: patch that fixes petervs and Pikes comments

Comment 7

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

Comment on attachment 51768 [details] [diff] [review]
patch that fixes petervs and Pikes comments

got r=Pike over mail

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 51768 [details] [diff] [review]
patch that fixes petervs and Pikes comments

sr=jst

Comment 9

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

attachment 51768 [details] [diff] [review] is checked in. Keeping the bug open since there is more stuff 
that needs precedence

Comment 10

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

pushing

Comment 11

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

pushing :(

Comment 12

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

...

Comment 13

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

fixed with the checkin of bug 117658

Comment 14

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

fixed with the branchlanding

Comment 15

[Axel Hecht]

mass verifying