getCurrentEditor() needs a tweak for efficiency

RESOLVED WONTFIX

Status

--
trivial
RESOLVED WONTFIX
14 years ago
14 years ago

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

14 years ago
Mozilla Composer makes a lot of calls to GetCurrentEditor() and particularly 
GetCurrentEditorElement().  The former calls the latter every time.  Both also 
make calls to C++ code.

The redundancies can be avoided by setting global variables after calling them 
once, and checking those same variables upon entering the functions involved.  
I found no indications where an editor element might be inserted by DOM in 
Mozilla chrome.  I also checked for all instances of "<editor" 
and "<xul:editor" in Mozilla chrome.  Again, only Composer and Mozilla 
Messenger use that particular element, and only once does Mozilla use more than 
one such element in an given XUL document.

Patch coming up in a few minutes; I tested it in Mozilla Composer, and there 
appear to be no regressions from it.  I have not tested it in Mozilla Messenger 
or Mozilla Thunderbird.  I have a sneaking suspicion this may slightly improve 
Composer response time (not enough for the human user to notice, though).
(Assignee)

Comment 1

14 years ago
Created attachment 150161 [details] [diff] [review]
patch, v1
(Assignee)

Updated

14 years ago
Attachment #150161 - Flags: review?(akkzilla)
> also checked for all instances of "<editor" 
> and "<xul:editor" in Mozilla chrome.  Again, only Composer and Mozilla 
> Messenger use that particular element, and only once does Mozilla use more than 
> one such element in an given XUL document.

Cc:ing mkaply@us.ibm.com.

Wow. Supposing that no other XUL client will ever insert editor tags using the
DOM or use only one editor tag per app is a very _very_ dangerous assertion...
The whole GetEditorElement() thing was made to allow multiple editor instances
per window/app, for instance in order to edit framesets or have multiple
editable areas in the same canvas. I would also like to say that Nvu uses more
than one editor tag per window *and* it DOES insert editor instances using
the DOM.
(Assignee)

Comment 3

14 years ago
Eww.  I did not realize that had changed with Nvu.  If that's the case, then we 
need to nix this whole bug in one big hurry.
(In reply to comment #3)

> Eww.  I did not realize that had changed with Nvu.  If that's the case, then we 
> need to nix this whole bug in one big hurry.

Let it be...
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → WONTFIX

Updated

14 years ago
Attachment #150161 - Flags: review?(akkzilla) → review-
(Assignee)

Comment 5

14 years ago
Here's a thought I had a little later, though.  (If you disagree, there's no 
need to reply and/or reopen this bug.)

Suppose we also included a couple of event listeners (DOMNodeInserted, 
DOMNodeRemoved) to watch for changes to the main XUL application's contents.  
If we did catch one of these events, we could then reset the gCurrentEditor* 
values to null.  This would then force the application to re-retrieve the 
correct nodes.

The only difficulty I would see there is in ensuring that we set the conditions 
for resetting to null properly.  Composer doesn't seem to add nodes to the 
edited document by DOM (at least, the event propagation for DOMNodeInserted 
doesn't go through the parent application for some reason).  It should, so we'd 
want to restrict event.target.ownerDocument to the XUL application.  We do get 
DOMNodeInserted and DOMNodeRemoved events fired for a bunch of toolbarbutton 
nodes designating which HTML element is being edited (next to the application 
icons at the bottom of the screen).  So to me it would make sense to apply the 
event listener to the appContent box, in this case only.  The question would 
then become: is this the most efficient, most cross-platform manner?

I toyed with the idea of checking to see specifically if an <xul:editor/> 
element was being added, but that becomes very complex in a hurry.  First, 
you'd have to check the node being added.  Then you'd have to check the 
getElementsByTagNameNS() bit.  Then you'd have to check anonymous nodes of 
every element being thrown in there, and their anonymous nodes... by the time 
you reach that, the speed reduction I originally implemented is burned off.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.