make inline history more efficient when inserting elements

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
Extensions: InlineHistory
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

Production
x86
Mac OS X
Dependency tree / graph

Details

(Assignee)

Description

5 years ago
from Wladimir Palant, in bug 919836 comment 9:

Now that I've looked into the Bugzilla code (https://bugzilla.mozilla.org/extensions/InlineHistory/web/inline-history.js) my suspicion in comment 8 seems correct:

>          var itemHtml =  '<div class="ih_history_item ' + containerClass + '" '
>                          + 'id="h' + i + '">'
>                          + item[3]
>                          + '<div class="ih_history_change">' + item[2] + '</div>'
>                          + '</div>';
>
>          if (ih_activity_sort_order == 'oldest_to_newest') {
>            currentDiv.innerHTML = currentDiv.innerHTML + itemHtml;
>          } else {
>            currentDiv.innerHTML = itemHtml + currentDiv.innerHTML;
>          }

This is being done for each single history item and in case of bug 705294 currentDiv is always the same element (there is only one comment so all history items are being inserted into the same place). Each time an item is being added the HTML code for the entire block (including all the previously added items) has to be reparsed. The most trivial way to avoid this issue would be replacing the code above by one that doesn't touch already existing elements:

>          var item = document.createElement("div");
>          item.className = "ih_history_item " + containerClass;
>          item.id = "h' + i;
>          item.innerHTML = item[3] + '<div class="ih_history_change">' + item[2] + '</div>';
>
>          if (ih_activity_sort_order == 'oldest_to_newest') {
>            currentDiv.appendChild(item);
>          } else {
>            currentDiv.insertChild(item, currentDiv.firstChild);
>          }
(Assignee)

Comment 1

5 years ago
this did indeed make a significant difference when displaying bug 705294, thanks again wladimir :)

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified extensions/InlineHistory/web/inline-history.js
Committed revision 9038.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 2

5 years ago
Wow, that's fast! (It really annoyed me that debug builds would pop up slow script warnings on relatively small pages such as bug 61491.)

Updated

5 years ago
Depends on: 921032

Updated

5 years ago
No longer depends on: 921032
(Assignee)

Updated

5 years ago
Blocks: 921133
You need to log in before you can comment on or make changes to this bug.