Closed
Bug 702642
Opened 13 years ago
Closed 12 years ago
DOMTemplate is relatively slow when evaluating JS ${}
Categories
(DevTools :: General, defect, P2)
DevTools
General
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 11
People
(Reporter: miker, Assigned: jwalker)
Details
Attachments
(1 file)
4.95 KB,
patch
|
dcamp
:
review+
|
Details | Diff | Splinter Review |
DOM Templater is relatively slow when evaluating JS ${}. Joe says he has a fix for this.
Assignee | ||
Comment 1•13 years ago
|
||
I'll also pre-compile the regexes while I'm at it.
Assignee: nobody → jwalker
Priority: -- → P2
Summary: DOM Templater is relatively slow when evaluating JS ${} → DOMTemplate is relatively slow when evaluating JS ${}
Comment 2•13 years ago
|
||
Would it make sense to have different syntax to explicitly opt-in to an eval rather than a property lookup?
Assignee | ||
Comment 3•13 years ago
|
||
Here's what I'm working on: Old: new Templater().processNode(node, data); New: template(node, data, options); Where the latter just does 'new Templater(options).processNode(node, data);' behind the scenes. There is a point to having an object (it allows you to cancel a asynchronous operations, etc). Previously you had to do: var t = new Templater(); t.processNode(node, data); Now: var t = template(node, data); If 'new Template()' is called without any options, then it sets allowEval:true for backwards compat, however the default for the template() method is allowEval:false
Assignee | ||
Comment 4•13 years ago
|
||
Mike / Dave - would be grateful for some feedback/review of the changes to DOM Template that we recently discussed: For the commits, see https://github.com/joewalker/domtemplate/commits/master for 'Nov 16, 2011'.
Assignee | ||
Updated•13 years ago
|
Status: NEW → ASSIGNED
Comment 5•13 years ago
|
||
Those changes look good to me.
Reporter | ||
Comment 6•13 years ago
|
||
Look fine to me too
Assignee | ||
Comment 7•13 years ago
|
||
Should be automatic given the comments expressed so far
Attachment #575482 -
Flags: review?(dcamp)
Updated•13 years ago
|
Attachment #575482 -
Flags: review?(dcamp) → review+
Assignee | ||
Comment 8•13 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=28134b5df88a
Assignee | ||
Comment 9•13 years ago
|
||
Failures in previous try were not down to this patch, but just in case: https://tbpl.mozilla.org/?tree=Try&rev=587bd6d86427
Comment 10•13 years ago
|
||
Is this ready to land?
Updated•13 years ago
|
Whiteboard: [land-in-fx-team]
Assignee | ||
Comment 11•13 years ago
|
||
Green on try: https://tbpl.mozilla.org/?tree=Try&rev=058542478c55 This could land if it's important for it to do so, but landing early could cause some unnecessary patch churn, which is slightly annoying for me in a separate repo.
Whiteboard: [land-in-fx-team]
Assignee | ||
Comment 13•13 years ago
|
||
https://tbpl.mozilla.org/?tree=Fx-Team&rev=cf3ed4316481
Whiteboard: [awaiting-go] → [fixed-in-fx-team]
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f1304b596193
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 11
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•