thervh70 / ContextProject_RDD

1 stars 0 forks source link

134 push entire dom #157

Closed mpsijm closed 8 years ago

mpsijm commented 8 years ago

Will close #134

Changes:

If there's anything wrong with anything, tell me! :D Else, please merge :)

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.108% when pulling a68bbc3cb18933e300b8297b65b85706ebf21308 on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.186% when pulling 34f8564767235a23b7b799912ac8b0062c0d0f18 on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

mdingjan commented 8 years ago

The additional lag that this feature takes with him is really noticable. xD But that's something we already expected, so that's fine. ;)

mdingjan commented 8 years ago

The status of the extension gets quite buggy after switching a few pull requests, which could have two causes in my opinion:

I think the last one is most likely the cause.

Exclaminator commented 8 years ago

Nice coding style, LGTM.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.126% when pulling 2e085fb7c05a5f25dfa3072d519754efe205c568 on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

mdingjan commented 8 years ago

The bug has indeed been fixed; thanks! :D I've played around more and can't find any more bugs, nice! :) The functionality is there and it has been tested well now, so good job! :D

Make sure to fix the typo and you have my approval for merging this PR. :)

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.126% when pulling 4e417268cf3e8002db8765f547f9e2364cc0a8fd on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

MathiasMeuleman commented 8 years ago

There isn't any possibility of making this faster right? The only problem is that I'm suddenly getting 400 statuscodes while typing this message, maybe something to look into? (You seem to have broken some KeystrokeEvents)

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.117% when pulling afecc55210c71b746320aff695b824c6cf93fae5 on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

mpsijm commented 8 years ago

Nope, this part cannot be made faster. Mainly because the rewriting of the DOM has to happen in the content script, because the page cannot be reconstructed in the background page. The 400 status codes are because the space characters cannot be pushed to the database. I'll poke Aaron about that :P

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.117% when pulling 1eac6e6f8b634f4f0eafd61fc07ecaa195dc7457 on 134-push-entire-dom into 818c5a7662dfcdbc0e82a8b7fd4617117a672ddb on dev.

MathiasMeuleman commented 8 years ago

Thanks for the rewrite, this does make it somewhat faster :) I will merge this