#16 trim weblabels data
Closed 3 years ago by gioma1. Opened 3 years ago by billauger.

@@ -41,7 +41,7 @@ 

          doc.head.appendChild(doc.createElement("base")).href = baseURL;

        }

        let firstURL = parent => parent.querySelector("a").href;

-       let allURLs = parent => Array.map(parent.querySelectorAll("a"), a => a.href);

+       let allURLs = parent => Array.map(parent.querySelectorAll("a"), a => a.href.trim());

        for (let row of doc.querySelectorAll("table#jslicense-labels1 tr")) {

          let cols = row.querySelectorAll("td");

          let scriptURL = firstURL(cols[0]);

this maybe the fix for what was reported on the mailing list?[1]?

[1]: http://lists.gnu.org/archive/html/bug-librejs/2018-09/msg00015.html

Nope, it's not. Please notice the patch tries to trim DOM nodes, not strings.
And that licenses are matched by canonical URLs, rather than by name.
Furthermore, it seems the reporter was talking about inline (legacy) license tags, so the patch, if needed, should apply somewhere else.

ok dont laugh i was tired yesterday - this was mainly just to point to
the correct part of the code

so now that i look at this, how can this ever fail in the way described
on the mailing list? - AFAIK the .href is always returned trimmed - i
was presuming that if trailing spaces problematic then this had to
be reading text node and not an anchor

in other words this current version of this patch should have no effect

a.href.trim()

so was that person imagining things? maybe this bug does not actually exist?

rebased onto f815fd3

3 years ago

so now that i look at this, how can this ever fail in the way described
on the mailing list
It cannot, in facts.
But inline license labels (which I did not touch at all so far) might.
However, before we keep investigating, it would be nice to see some live examples of failing pages, otherwise we're shooting in the dark.

yes as i said the main reason i put this up so hastily was to link to
it on the mailing list in hopes the OP would respond with an example

ah yes i see now - the OP wrote that the problem was with "inline labels"

I got it to work with inline labels, but I notice that LibreJS is very sensitive
and fails to recognize a license name if there's a space after the name.

are you reading the mailing list Gorgio? it has gotten a lot of activity this
week for some reason

are you reading the mailing list Gorgio? it has gotten a lot of activity this
week for some reason
No I'm not. I've noticed it and read the thread because you've opened this PR.
Let's keep it this way :)
If something that looks like a real bug arises, an issue can be opened here with proper steps to reproduce, and I'll do my best to fix it ASAP.

The increased activity might be people excited at the new releases (7.15 and 7.16), I guess.

ok that person just replied with a different error after using a weblabels table instead - i have not verified it so i dont know if it needs a new issue

Error fetching Web Labels at  
<a href="javascript" rel="jslicense">
 TypeError: "parent is undefined, can't access property "querySelectorAll" of it"
  allURLs moz-extension://81a2b90c-80b5-4f43-a457-bfb27a41eabc/content/externalLicenseChecker.js:44:31
    fetchWebLabels moz-extension://81a2b90c-80b5-4f43-a457-bfb27a41eabc/content/externalLicenseChecker.js:48:27

the problem with using pagure as the main issue tracker is that only the devs know about this repo - it is not advertised publicly - there is also one on github but it is only used as a mirror - the bug reports will all come in on the mailing list and and savannah - this pagure repo was only created only for the purpose of code review - so to use this as an issue tracker would require that someone needs to triage the bug reports from the maling list and savannah then open issues here - i can not guarantee that i will always have time to do that

it would probably be best to continue using savannah as the main issue tracker unless this repo were advertised as accepting bug reports from the public - we can discuss that but again this issue tracker is not the best pace to do that - that is what the mailing list is for

there is one other potential bug reported here against the previous version:
http://lists.gnu.org/archive/html/help-librejs/2018-08/msg00003.html

with a follow-up here using the latest code
http://lists.gnu.org/archive/html/help-librejs/2018-08/msg00005.html

again this would probably be a separate issue than this one - and new issues should probably be opened on savannah for now and to reserve pagure for pull-requests/code-review unless it is decided otherwise

FWIW - i would say to choose only one or the other to accept bug reports from the public - either the pagure issue tracker or the savannah issue tracker but not both

http://lists.gnu.org/archive/html/help-librejs/2018-08/msg00005.html

The bug seems their (or of the documentation): the URL "https://directory.fsf.org/wiki/License:Expat" they provide for Expat is nowhere to be found in librejs' license definitions data.

then what is the correct URL for expat that librejs will accept ? is it MIT? or X11 maybe?

TypeError: "parent is undefined, can't access property "querySelectorAll" of it"

This, AFAICS, can only happen they're giving us a malformed table to parse (specifically, one with no second column). It doesn't seem a bugs of ours, even though we could be more lenient and informative.

we can discuss that but again this issue tracker is not the best pace to do that - that is what the mailing list is for

OK, I'll try to subscribe it, but at this moment I'm very focused on coding and pushing stuff out of the door, so my chatting bandwith is extremely limited.

then what is the correct URL for expat that librejs will accept ? is it MIT? or X11 maybe?

http://www.jclark.com/xml/copying.txt

And that's what the docs says too

ok so the exact URL in column 2 is arbitrary - it does not need to be
a canonical URL - it just needs to point to a copy of the license - i
was under the impression that there was only one possible URL for each
license type

Pull-Request has been closed by gioma1

3 years ago
Metadata