Closed Bug 78068 Opened 23 years ago Closed 23 years ago

xsl:import not supported

Categories

(Core :: XSLT, defect, P2)

Product:
Core
Component:
XSLT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.5

People

(Reporter: sicking, Assigned: sicking)

References

Details

Attachments

(9 files)

patch ver 1
29.00 KB, patch
Details | Diff | Splinter Review
ver 2
30.00 KB, patch
Details | Diff | Splinter Review
implement default templates using code instead modifying DOM
8.30 KB, patch
Details | Diff | Splinter Review
Move retreaveDocument from DocumentFunctionCall to ProcessorState
9.33 KB, patch
Details | Diff | Splinter Review
small change (and small fix) to ProcessorState::findTemplate to account bottom-to-top processing in processTopLevel
1.36 KB, patch
Details | Diff | Splinter Review
actually implement xsl:import
14.29 KB, patch
Details | Diff | Splinter Review
Merged with tip. Only doing the "total" file for now as there are no functional differences...
30.26 KB, patch
Details | Diff | Splinter Review
With importContainers
25.38 KB, patch
Details | Diff | Splinter Review
fixed comments from peterv and Pike
29.43 KB, patch
shaver
: review+
shaver
: superreview+
Details | Diff | Splinter Review 
xsl:import is not supported in Transformiix. Will attach an patch that
implements it by doing the following:

* Adds some code to the current xsl:import to handle import precidence
   Added the following functions
    ProcessorState::getImportPrecedence gets current import precedence
    ProcessorState::setImportPrecedence sets current import precedence
    ProcessorState::newImportPrecedence sets current import precedence to that
                                        of a new imported stylesheet
   Changed code in
    XSLTProcessor::processTopLevel

* Revese the order in which top-level elements are process (ie do it lastChild 
  to firstChild). I had to do this for the importprecedence calculations.
   Changed code in
    XSLTProcessor::processTopLevel

* Made ProcessorState able to load documents. This code is basically moved here
  from DocumentFunctionCall. Now document() and xsl:import/include uses the same
  mechanism to load documents.
   Moved the following function
    DocumentFunctionCall::retrieveDocument to ProcessorState::retrieveDocument
   Changed code in
    DocumentFunctionCall::evaluate
    ProcessorState::~ProcessorState
   Removed the following functions
    ProcessorState::getLoadedDocument
    ProcessorState::addLoadedDocument

* Added made correct recursion-loop-protection for xsl:import/include.
   Added the following functions
    ProcessorState::enterStylesheet   notifies the processorstate that a new
                                      stylesheet is entered
    ProcessorState::leaveStylesheet   notifies the processorstate that a
                                      stylesheet is left

* The bottom-to-top processing didn't go well with how default templates worked
  which actually is since findTemplate dosn't support importprecidence. However
  I didn't want to fix this in findTemplate since our defaulttemplate handling
  is pretty bad anyway. We currently do it by adding the templates to the
  xslt-DOM. This is first of all a BadThing, and second, not all default
  templates can be done this way.
  Now I instead implement them by adding some code to ::processTemplate
   Removed code from
    ProcessorState::initialize
    ProcessorState::~ProcessorState
   Added code in
    XSLTProcessor::processTemplate

* Some small changes in findTemplates to account for the top-to-bottom search
  When two templates have the same priority I select the first one instead of
  the last one.
   Changed code in
    ProcessorState::findTemplate

I'm not entierly happy with how import states work (don't like adding three
functions and two variables to ProcessorState), but lets take that over IRC.

Ok, with a description like this I expect a fast review ;-)
Just noticed some not-so-nice if nesting in the new default template code, but 
I'll fix that when we've decided about how to handle import precedence
Status: NEW → ASSIGNED
Keywords: review
removed get/setImportPrecedence from ProcessorState and made import precedence 
an argument to XSLTProcessor::processTopLevel instead. So the flow of the code 
is exactly the same as before...
I split up the patch for your reviewing pleasure :)

The patch that actually implements xsl:import is very dependent on the other 
patches, but the other ones should be pretty independent...
two things, enterStylesheet doesnt push the URI to enteredStylesheets, and
there's a TAB in the line impPrec = importPrecedence;

Apart from this the singled patches looked fine.

Gonna check the global patch later.

Axel
about implementing the import precedence.
all we needed was moving ProcessorState.templates from a NodeList to a list of
NodeLists, and we would get the right behaviour in findTemplate rather easy.
Having just a list of precedence yields the idea of dropping the numeric value
alltogether, and it seems like a feasable task.
Then we could do processTopLevel forward again, which is a good investment into
the future, where we might end up doing at least processTopLevel async.

Axel
spawned bug 83651 for making use of import precedence.
I have two major points.
First, we shouldn't implement import as part of the processTopLevel children
loop.
It's only allowed as first elements of xsl:stylesheet, and so we should impl
import in a extra loop.
Going with the idea of frames, adding the current frame when entering the 
toplevel loop and after processing the imports should get us to an easy (forward
looping) implementation of import precedence.
I'm rather religious on both points, do have only forward loops, and don't allow
xsl:import mixed with other loplevel elements. As far as I read the spec, we
are in worse trouble than we have to, if we don't keep those two loops apart. 

Axel
Blocks: 83651
ok, I've implemented the frames (though I call them containers) and made a 
separate loop for imports. Since I now traverse the document top-to-bottom I'm 
no longed dependant on default templates being implemented in code so I broke 
out bug 86270 on that.

However this is now dependant on a bugfix in the List class which is avalible 
in bug 85189. If that is not landed first the list of containers will be broken 
(which currently "only" causes leaks)
adding dep on loading documents at all for module
Depends on: 73936
Priority: -- → P5
mental note, ProcessorState::loadedDocuments
setting milestone on the bugs that nisheeth wanted. Setting Priority up for
landing indication

Axel
Priority: P5 → P2
Target Milestone: --- → mozilla0.9.4
pushing and cc'ing shaver for sr
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Comment on attachment 46069 [details] [diff] [review]
fixed comments from peterv and Pike

sr=shaver (also marking has-review based on previous comment)
Attachment #46069 - Flags: superreview+
Attachment #46069 - Flags: review+
Small crashlanding, but now it's in...
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Keywords: review
Resolution: --- → FIXED
bitching buttons, verfication spam
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: