After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769363 - [PATCH] GXmlElement : fix order in DomHTMLCollection for get_elements_by_class_name
[PATCH] GXmlElement : fix order in DomHTMLCollection for get_elements_by_clas...
Status: RESOLVED FIXED
Product: gxml
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GXml maintainer(s)
GXml maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-07-31 17:29 UTC by Yannick Inizan
Modified: 2016-09-26 11:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GXmlElement : change order in collections. (1.42 KB, patch)
2016-07-31 17:29 UTC, Yannick Inizan
none Details | Review
GXmlElement : change order in collections. (2.24 KB, patch)
2016-07-31 20:30 UTC, Yannick Inizan
none Details | Review
get_elements_by_class_name test (313 bytes, text/x-vala)
2016-09-01 09:05 UTC, Yannick Inizan
  Details
get_elements_by_class_name test html file (141 bytes, text/html)
2016-09-01 09:05 UTC, Yannick Inizan
  Details
[PATCH] get_elements_by_class_name : return nodes according to specification. (2.07 KB, patch)
2016-09-01 09:45 UTC, Yannick Inizan
none Details | Review
GXmlElement : fix order in DomHTMLCollection for get_elements_by_class_name. (2.09 KB, patch)
2016-09-14 01:53 UTC, Yannick Inizan
none Details | Review
HTML test file (221 bytes, text/html)
2016-09-14 01:53 UTC, Yannick Inizan
  Details

Description Yannick Inizan 2016-07-31 17:29:26 UTC
Created attachment 332453 [details] [review]
GXmlElement : change order in collections.

GXml add children collections before current node while webbrowser DOM do the opposite
Comment 1 Yannick Inizan 2016-07-31 20:30:30 UTC
Created attachment 332454 [details] [review]
GXmlElement : change order in collections.

get_elements_by_class_name : add child node only if class attribute has all required class (and don't add node for each class found).
Comment 2 Daniel Espinosa 2016-08-01 00:46:46 UTC
Review of attachment 332454 [details] [review]:

Is very important to patch both source code and unit tests. While I'm trying to correctly interpret DOM4 specification, most miss understanding will rise, please make sure you make GXml behave as expected and test it.
Comment 3 Daniel Espinosa 2016-08-01 18:54:19 UTC
Please base your patch against latest commit:

https://git.gnome.org/browse/gxml/commit/?id=936565d0e32a16a5b03f5dac0835574f2c33a5f8

Thanks again for all your help.
Comment 4 Daniel Espinosa 2016-08-29 01:06:30 UTC
get_elements_by_tag_name() adding first current element has been fixed at commit:

https://git.gnome.org/browse/gxml/commit/?id=88dd65655b53224521d71d9b7914de77985ab036
Comment 5 Daniel Espinosa 2016-08-29 03:52:19 UTC
get_elements_by_class_name() now returns only nodes with same classes in search, no matter its order. Unit Tests was updated.

This commit fix this bug. Please, close it.

https://git.gnome.org/browse/gxml/commit/?id=cf0f1116f4c0a2751ed3e835f4bd37e09c954256
Comment 6 Yannick Inizan 2016-09-01 09:04:44 UTC
no. get_elements_by_class_name must return each node which has this class.
please run example provided as attachment. 

node.getElementsByClassName in browsers returns all node
Comment 7 Yannick Inizan 2016-09-01 09:05:28 UTC
Created attachment 334591 [details]
get_elements_by_class_name test
Comment 8 Yannick Inizan 2016-09-01 09:05:58 UTC
Created attachment 334592 [details]
get_elements_by_class_name test html file
Comment 9 Yannick Inizan 2016-09-01 09:45:45 UTC
Created attachment 334593 [details] [review]
[PATCH] get_elements_by_class_name : return nodes according to specification.
Comment 10 Yannick Inizan 2016-09-01 09:49:58 UTC
btw, get_elements_by_tag_name works. Thanks.
Comment 11 Daniel Espinosa 2016-09-07 23:23:42 UTC
(In reply to Yannick Inizan from comment #8)
> Created attachment 334592 [details]
> get_elements_by_class_name test html file

Your example just include one class to search for, but according to DOM specification at:

https://www.w3.org/TR/dom/#concept-getelementsbyclassname

We should:

"Return a HTMLCollection rooted at root, whose filter matches descendant elements that have all their classes in classes"

The sentence *all their classes in classes*, I understand that if you search for "class1 class2" will match any with "class1" or "class2" and "class1 class2", this is if you have at least one of the searched classes node match if node have less classes, but if you have equal should match if all of them are in search, and if node have more it should match if it has all on search.

Is it correct?
Comment 12 Daniel Espinosa 2016-09-08 14:25:55 UTC
I think this issue should be fixed at:

https://git.gnome.org/browse/gxml/commit/?id=1e60cdf5387c619084f2fc563e69018116f72be6

please confirm. If so please close this bug.
Comment 13 Daniel Espinosa 2016-09-08 14:27:24 UTC
Sorry I missed your credit, but I still will add tests to strip spaces in classes, then I'll add your credit about your work in latest commit.
Comment 14 Yannick Inizan 2016-09-14 01:53:20 UTC
Created attachment 335475 [details] [review]
GXmlElement : fix order in DomHTMLCollection for get_elements_by_class_name.
Comment 15 Yannick Inizan 2016-09-14 01:53:44 UTC
Created attachment 335476 [details]
HTML test file
Comment 16 Daniel Espinosa 2016-09-14 17:52:10 UTC
Please check latest commit, it should fix your issue.

https://git.gnome.org/browse/gxml/commit/?id=920d53fca8b60807eef8887115bc2a4a2bf70f70

If fixed, please close this bug in order to continue to make a 0.12 release.
Comment 17 Yannick Inizan 2016-09-26 11:25:37 UTC
fixed. thanks