Skip to content
This repository has been archived by the owner on Jul 11, 2024. It is now read-only.

Apply the new innerHTML only if the text have actually changed #3

Merged
merged 3 commits into from
Nov 4, 2014

Conversation

prusse-martin
Copy link
Contributor

This will avoid unnecessary HTML parses and DOM rendering.

It is intended as a partial solution to #2.

It does not fully address the problem in that issue since when new content arrives it is possible that the selection is lost.
But it offers a performance improvement when the contents of the console are massive due to reduced HTML parsing and DOM re-rendering.

…ding unnecessary HTML parses and DOM rendering
var text_changed = (original_inner_html.length != reformated_inner_html.length);
if (text_changed) pre_element.innerHTML = reformated_inner_html;

pre_element = original_inner_html = reformated_inner_html = text_changed = undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems to be useless because they are local variables, no need to reset them.

@fdubost
Copy link
Member

fdubost commented Nov 3, 2014

👍

@KuiKui
Copy link
Contributor

KuiKui commented Nov 4, 2014

@fdubost tu l'as testé sur ton Jenkins ?

@fdubost
Copy link
Member

fdubost commented Nov 4, 2014

Oui @KuiKui, j'ai testé

@KuiKui
Copy link
Contributor

KuiKui commented Nov 4, 2014

Tu pourrais me remettre en project collaborators stp ?
Histoire que je puisse merger les PR 😸

KuiKui added a commit that referenced this pull request Nov 4, 2014
Apply the new `innerHTML` only if the text have actually changed
@KuiKui KuiKui merged commit 742b639 into BedrockStreaming:master Nov 4, 2014
@KuiKui
Copy link
Contributor

KuiKui commented Nov 4, 2014

The new version 0.0.4 will be available soon on Chrome Web Store.

@prusse-martin prusse-martin deleted the less-intrusive-update branch November 5, 2014 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants