search

vaadin / serverside-elements

Vaadin add-on that makes it easy to use Web Components or any HTML elements from Vaadin Framework
https://vaadin.com/addon/elements-add-on
Apache License 2.0
14 stars 12 forks source link

2 Bugfixes for ContextImpl and RootImpl #22

Closed mpetris closed 5 years ago

mpetris commented 5 years ago

Hi, this Pull Request suggests two bugfixes:

ContextImpl: in the case of a LeafNode calling childNodes throws an Exception, therefore we check childNodeSize first

RootImpl: access to index 2 fails for elements with no children and no attributes because array size is 2 then containing only tagname and element id, therefore we check for length before trying access at idx 2

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

Legioth commented 5 years ago

Hi. Thank you for making these fixes. It would be really awesome if you could also add unit tests for these cases. I can also do that even though I won't have time to get that done immediately.

mpetris commented 5 years ago

Hi @Legioth, ok I added a test for each fix. In order to make the second test addLeafToRootNode actually relevant I needed to upgrade the referenced Vaadin version to 8.6.2 in the pom files. This is because the addressed problem only occurrs with the newer jsoup version 1.11.2 referenced by Vaadin 8.6.2 and not with the older 1.8.1 referenced by Vaadin 8.0.5.

Legioth commented 5 years ago

Thanks again for contributing these fixes!

Legioth commented 5 years ago

Released as https://vaadin.com/directory/component/elements-add-on/0.2.1