Add support for only one last `@supportURL`, fix naming standardization, and such #189

Martii · 2014-06-19T11:37:54Z

NOTE: It appears that Zren is using .meta to be the actual metadata block item names and so any manipulation of these should be stored elsewhere just in case @collaborators, @homepages, @supportURLS, etc. is ever used/supported directly in a metadata block... doubt it will based off of Anthony's prior responses for other keys but does make the code here simpler to read... and we don't need to be potentially overwriting user specified keys even if they don't really exist anywhere.

Applies to:

@supportURL is a pseudo unique key (e.g. non-upmixed) in our context as per the implied concern, especially at #96 (comment), around that issue. Authors can of course specify more at any time in their source but we only display/recognize the last one like GM standards for conflict resolution keys (and Scriptish probably already does for this exact key). I don't know of anyone yet that would want to have to check multiple places for issues. We could, at a future date, alter our issue tracker linkage if a user doesn't specify us as the primary. Some thoughts to ponder on... but that's another pr/issue down the line perhaps.

Tested on dev at this script using this metadata block:

// ==UserScript==
// @name          RFC 2606§3 - @license and @licence Unit Test
// @namespace     http://localhost.localdomain
// @description   Tests out non-unique keys
// @copyright     2007+, Marti Martz (http://userscripts.org/users/37004)
// @license       GPL version 3 or any later version; http://www.gnu.org/copyleft/gpl.html
// @licence       (CC); http://creativecommons.org/licenses/by-nc-sa/3.0/
// @version       2014.06.19.1
// @icon          https://s3.amazonaws.com/uso_ss/icon/13701/large.png

// @include   http://www.example.com/*
// @include   http://www.example.net/*
// @include   http://www.example.org/*

// @author Marti
// @collaborator jerone
// @collaborator sizzle
// @collaborator Zren

// @homepageURL https://openuserjs.org/scripts/marti/httplocalhost.localdomain/RFC_2606%C2%A73_-_license_and_licence_Unit_Test
// @homepage    http://localhost:8080/scripts/marti/httplocalhost.localdomain/RFC_2606%C2%A73_-_license_and_licence_Unit_Test
// @supportURL  http://example.com
// @supportURL  https://openuserjs.org/scripts/marti/httplocalhost.localdomain/RFC_2606%C2%A73_-_license_and_licence_Unit_Test/issues
// @supportURL  http://localhost:8080/scripts/marti/httplocalhost.localdomain/RFC_2606%C2%A73_-_license_and_licence_Unit_Test/issues

// ==/UserScript==

…ation **NOTE**: It appears that Zren is using `.meta` to be the actual metadata block items and so any manipulation of these should be stored elsewhere just in case `collaborators`, etc. is ever used/supported in a metadata block... doubt it will based off of Anthony's prior responses but does make the code simpler to read. Applies to: * OpenUserJS#174 * OpenUserJS#161

Applies to OpenUserJS#189

sizzlemctwizzle · 2014-06-20T09:13:06Z

views/pages/scriptPage.html

@@ -35,11 +35,12 @@
                  </ul>
                </span>
              {{/script.hasGroups}}
-              {{#script.meta.homepages}}<p><i class="fa fa-fw fa-home"></i> <b>Homepage:</b> <a href="{{name}}">{{name}}</a></p>{{/script.meta.homepages}}
+              {{#script.homepages}}<p><i class="fa fa-fw fa-home"></i> <b>Homepage:</b> <a href="{{name}}">{{name}}</a></p>{{/script.homepages}}
+              {{#script.support}}<p><i class="fa fa-fw fa-support"></i> <b>Support:</b> <a href="{{script.support}}">{{script.support}}</a></p>{{/script.support}}


@supportURL javascript:alert(document.cookie);

Although mailto: might be a nice protocol to keep.

We'll need to add a validation filter to all linkified ones then... not just @supportURL. I'll be away for this weekend btw. So a new issue towards Munday?

…onent` Output HTML on dev: * `<a href="/users/%3Ci%3Edoes%20this%20italicize%3C%2Fi%3E"><i>does this italicize</i></a>` ... instead of HTML entity escaping

Applies to OpenUserJS#189 * Add sanitize-html for `@supportURL` and `@homepageURL` parts * Sanitize those urls using a html stub encasement... only push if exactly equal. e.g. don't show if they aren't * Create html whitelist link json for this since it's used multiple times * Change affected view page to accomodate changes Tested on dev okay but needs testing verification

Martii · 2014-06-23T03:36:04Z

@sizzlemctwizzle
Is this better? If not say why and how you, or anyone else, would do it differently please. :) mailto: is supported on linked @*URLs as requested.

@OpenUserJs/admin

@OpenUserJs/backend (interesting that I can't do this list)

@OpenUserJs/frontend (interesting that I can't do this list)

Everyone please remember to read my commit messages... they tell everyone what a commit is doing in plain english along with sensible variable names. TIA

Are there any other use cases anyone can think of to hack these values?
Encodings all correct? (I think I finally understand, with an epiphany, this weekend how mustache is using {{{ vs {{)

Tested on dev at this new script ... somehow I've lost access to the original script on both pro and dev. :\

Btw do we want a simple rel="nofollow" on any of these page generated links?

Another thing... I know V8 doesn't do block level disposable variables but should we be coding as if it may eventually? The difference is:

ES5ish

var tempIdentifier;

if (true) {
  tempIdentifier = 1;
  // Do something with tempIdentifier but not used anywhere else outside of this block
} else {
  tempIdentifier = 2;
  // Do something with tempIdentifier but not used anywhere else outside of this block
}

vs. future support of ES6 (Harmony)

if (true) {
  var tempIdentifier = 1;
  // Do something with tempIdentifier but not used anywhere else outside of this block
} else {
  var tempIdentifier = 2;
  // Do something with tempIdentifier but not used anywhere else outside of this block
}

... obviously in ES6 it would be a let instead of var ... but this does affect how we might code. Worst thing I've seen in ACE is ... already defined warning since it's still going off of ES5ish rules... nothing shown in console for warnings either since I'm currently using the latter. Our last edited style guide says this which is completely untrue in all browsers and backend JavaScript engines but is probably true in current dated V8's ES5ish implementation in node.js.

Since the full test script is currently local here.. here is the last known metadata block:

// ==UserScript==
// @name          RFC 2606§3 - @license and @licence (recovered) Unit Test
// @namespace     http://localhost.localdomain
// @description   Tests out non-unique keys
// @copyright     2007+, Marti Martz (http://userscripts.org/users/37004)
// @license       GPL version 3 or any later version; http://www.gnu.org/copyleft/gpl.html
// @licence       (CC); http://creativecommons.org/licenses/by-nc-sa/3.0/
// @version       2014.06.22.0
// @icon          https://s3.amazonaws.com/uso_ss/icon/13701/large.png

// @homepage     http://localhost:8080/scripts/Marti/RFC_2606%C2%A73_-_license_and_licence_%28recovered%29_Unit_Test
// @homepage     https://openuserjs.org/scripts/Marti/RFC_2606%C2%A73_-_license_and_licence_%28recovered%29_Unit_Test
// @homepage     javascript:alert('hello, world');
// @homepageURL  mailto:user@example.com

// @supportURL   http://localhost:8080/scripts/Marti/RFC_2606%C2%A73_-_license_and_licence_(recovered)_Unit_Test/issues
// @supportURL   javascript:alert('hello, world');
// @supportURL   mailto:user@example.com

// @include   http://www.example.com/*
// @include   http://www.example.net/*
// @include   http://www.example.org/*

// @author Marti
// @collaborator  sizzle
// @collaborator  Zren
// @collaborator  This User Better Not Exist
// @collaborator  <i>does this italicize or generate invalid encoded href</i>

// ==/UserScript==

jerone · 2014-06-23T14:53:58Z

libs/modelParser.js

+  if (script.meta.supportURL) {
+    if (_.isString(script.meta.supportURL)) {
+      var htmlStub = '<a href="' + script.meta.supportURL + '"></a>';
+      if (htmlStub === sanitizeHtml(htmlStub, htmlWhitelistLink)) {


Can you tell why you do this?

Can you tell why you do this?

See #189 (comment)

I was wondering more about technical why.

Only thing I could think of is if an inexperienced user would accidentally click on this type of link with a script and add an event listener to monitor keystrokes... Sanitizing it keeps it from doing this. Also sizzle mentioned the stealing cookie bit... so a cross-domain call or even within the same domain posting somewhere could be achieved. The html tag sanitization can be used in case anyone accidentally uses {{{text}}} and commits a bug with a potential security issue. So basically a two fold potential op here. If you have any other recommendations we don't have to use this. I am just fixing and cleaning up the bug introduced from the new UI in #103 and #129. Reusing existing objects (methods) already in memory will help the drones too. As I've mentioned before I'm not blaming because we all make mistakes and I propagated this one... and then sizzle caught it. I'm quite thankful for all the verification/testing provided. :)

Sorry, dyslectic person here. I meant what happens on this specific line? sanitizeHtml validates if htmlStub is valid html and you compare it to the original? What happens if the url contains a trailing space?

What happens if the url contains a trailing space?

It won't because the imported CI/iW/usoC routine should be trimming and we are using/storing a copy of the metadata block.

jerone · 2014-06-23T15:08:29Z

Fast review

Btw do we want a simple rel="nofollow" on any of these page generated links?

Yes. Outside urls should have rel="nofollow".

ES6 (Harmony)

-1 for me. Keep things simple and one standard.

cletusc · 2014-06-23T15:38:26Z

rel="nofollow"

For all outside links, yes, unless we whitelist certain domains (MDN, github, etc.).

ES6

I am also a -1 based on the massive amount of red on the kangax table.

Martii · 2014-06-23T15:41:54Z

Thanks guys. Okay give me a few moments and I'll move the var declarations way up near the top of the functions and deharmonize. :)

…in node.js and downgrade to ES5 standards, and [our current styleguide enforcement](OpenUserJS@0263c79#diff-ba2dc84c4bcbe54f368b6c7d3bc1b13eR39) Applies to OpenUserJS#189

* One other bug fix for single `@supportURL` usage... copy/paste goof Applies to OpenUserJS#189

Martii · 2014-06-23T16:52:39Z

Updated dev test script

unless we whitelist certain domains (MDN, github, etc.).

I recommend that we save this part for another issue upon user request. This gets more into SEO than I think we are ready for just yet (don't see any robots.txt either in our source)... plus we might be perceived as "taking sides" if there is ever a conflict between one user request and another. A discussion best suited for another issue I think. :)

Fixed both of those concerned issues... anything else?

…d set to `null` * See [here](OpenUserJS@0263c79#diff-ba2dc84c4bcbe54f368b6c7d3bc1b13eR28) Applies to OpenUserJS#189

jerone · 2014-06-23T20:28:46Z

Shouldn't we show @supportURL's that are not really links as normal text? Right now they are dismissed.

sizzlemctwizzle · 2014-06-23T20:34:40Z

If isn't a valid url, why should we list it as a support url?
On Jun 23, 2014 3:28 PM, "Jeroen van Warmerdam" notifications@github.com
wrote:

Shouldn't we show @supportURL's that are not really links as normal text?
Right now they are dismissed.

—
Reply to this email directly or view it on GitHub
#189 (comment)
.

Martii · 2014-06-23T20:36:30Z

Right now they are dismissed.

Showing invalid ones will involve more mustache logic. I opted out because it's just as easy to check the source for invalid entries... but what we do show with linking we should validate as much as possible... A variant of Count Issues (CI) will probably be rewritten eventually for OUJS for those looking for extreme specifics. (this is going to be interesting trying to do this on the homepage because it will probably trigger a script counter update :\ I will probably need to do this on Source Code page instead.) ... plus there is oujs - Meta View right now. This is nearing the bare minimum I think that we should show unless someone requests something else that we all agree upon.

jerone · 2014-06-23T20:55:38Z

Showing invalid ones will involve more mustache logic. I opted out because it's just as easy to check the source for invalid entries... but what we do show with linking we should validate as much as possible

That's fine.

This is nearing the bare minimum I think that we should show unless someone requests something else that we all agree upon.

I agree. Did found some useful ones at Scriptish... (Edit: reference issue #197 )

jerone · 2014-06-23T21:30:06Z

I did some fast reviewing of this PR and I'm +1 on merging.

Edit:
Hmmm I'm still thinking if using a regex wouldn't be more useful. This logic can then used for all metadata block items. e.g. @license and @copyright may also contain URLs.

Martii · 2014-06-23T22:43:53Z

using a regex wouldn't be more useful

Regular expressions (re) aren't always the answer for everything... sometimes more human readable methodologies in our base language of JavaScript are preferred instead of a syntax of its own using re. Which re is implemented is another factor... each programming language and even ES implementation sometimes have different rules.

show with linking

... e.g. @license and @copyright may also contain URLs.

There is no way to guarantee someone is going to use the @license properly so I'll never recommend linking that... plus the key is not suffixed with URL. I tried to get ppl to standardize on @license syntax a long time ago but they still put in whatever they want to and upstream GM dismissed any accountability on the matter. Same with @copyright. The mustache {{text}} uses HTML Entities so if it is ever code it won't get executed... that is our current check there. e.g. < becomes < in the DOM. Mustache uses the terminology of escaping which can mean a lot of things... they should prefix/suffix it with "un/escaping HTML Entities" for the triple curly braces. That is what threw me off about 30 days ago with mustache because it was unclear and very new to me. Escaping is different than HTML Entity de/referencing not to mention escape is deprecated and nearing eol in favor of encodeURI and encodeURIComponent. The general goal with auto-linking is if a user wants it shown with auto-linking they need to follow OUJS posting guidelines/recommendations. (to be inserted sometime in the wiki or TOS after this is implemented... like I said earlier nearing the bare minimum... and nearing the maximum too)

Add support for only one last `@supportURL`, fix naming standardization, and such

* Missed one in OpenUserJS#189 and applies to OpenUserJS#161

* Trap if a Userscript doesn't encode one of these urls correctly. Fixes "Page Load Error" with "Failed to Connect"... server trip Applies to OpenUserJS#819, OpenUserJS#239, OpenUserJS#197, and OpenUserJS#189 Ref: * https://openuserjs.org/discuss/webhook_not_working

Martii added the enhancement label Jun 19, 2014

Martii self-assigned this Jun 19, 2014

Martii added the bug label Jun 19, 2014

Fix missing url from paste op

3b13a3c

Applies to OpenUserJS#189

sizzlemctwizzle reviewed Jun 20, 2014
View reviewed changes

Martii mentioned this pull request Jun 20, 2014

Validate shown and linkified @keys #191

Closed

Martii added 2 commits June 22, 2014 19:11

Merge branch 'master' into addSupportURLsupport

28d1860

Fix @collaborator to generate the URI properly using `encodeURIComp…

39a1749

…onent` Output HTML on dev: * `<a href="/users/%3Ci%3Edoes%20this%20italicize%3C%2Fi%3E"><i>does this italicize</i></a>` ... instead of HTML entity escaping

Martii added the security label Jun 23, 2014

Martii added needs testing labels Jun 23, 2014

Martii changed the title ~~Add support for only one last @supportURL and fix naming standardization~~ Add support for only one last @supportURL, fix naming standardization, and such Jun 23, 2014

Martii added the sooner label Jun 23, 2014

jerone reviewed Jun 23, 2014
View reviewed changes

Martii added 2 commits June 23, 2014 09:55

Comply with lack of ES6 compliance in Google V8 dated specifications …

54e86bb

…in node.js and downgrade to ES5 standards, and [our current styleguide enforcement](OpenUserJS@0263c79#diff-ba2dc84c4bcbe54f368b6c7d3bc1b13eR39) Applies to OpenUserJS#189

Add rel="nofollow as default unless on current production

d817050

* One other bug fix for single `@supportURL` usage... copy/paste goof Applies to OpenUserJS#189

This was referenced Jun 23, 2014

One more exacting for .x ES versions ... #196

Merged

Fix for clarity usage of ES5 usage on block level identifiers #195

Merged

Comply with STYLEGUIDE.md not using unset but declared identifiers an…

1719f88

…d set to `null` * See [here](OpenUserJS@0263c79#diff-ba2dc84c4bcbe54f368b6c7d3bc1b13eR28) Applies to OpenUserJS#189

Martii referenced this pull request Jun 23, 2014

Sanitize the output from the block level markdown renderers.

23f47bc

Martii mentioned this pull request Jun 23, 2014

Metadata block items #197

Closed

Martii removed the question label Jun 24, 2014

sizzlemctwizzle added a commit that referenced this pull request Jun 24, 2014

Merge pull request #189 from Martii/addSupportURLsupport

f0c8c99

Add support for only one last `@supportURL`, fix naming standardization, and such

sizzlemctwizzle merged commit f0c8c99 into OpenUserJS:master Jun 24, 2014

Martii removed the sooner label Jun 25, 2014

Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Jun 26, 2014

Fix missing default script @license

01ac264

* Missed one in OpenUserJS#189 and applies to OpenUserJS#161

Martii mentioned this pull request Jun 26, 2014

Fix missing default script @license #207

Merged

cletusc mentioned this pull request Jun 27, 2014

Feature: Add "more" link on scripts in addon manager greasemonkey/greasemonkey#1944

Closed

Martii removed their assignment Jul 3, 2014

Martii deleted the addSupportURLsupport branch July 7, 2014 09:24

Martii removed the needs testing Anyone can add this but it is primarily there for the Assignee indicating that Testers are wanted. label Apr 27, 2015

Martii mentioned this pull request Mar 7, 2016

Post fix for @supportURL and @homepageURL #920

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for only one last `@supportURL`, fix naming standardization, and such #189

Add support for only one last `@supportURL`, fix naming standardization, and such #189

Martii commented Jun 19, 2014

sizzlemctwizzle Jun 20, 2014

Martii Jun 20, 2014

Martii commented Jun 23, 2014

jerone Jun 23, 2014

Martii Jun 23, 2014

jerone Jun 23, 2014

Martii Jun 23, 2014

jerone Jun 23, 2014

Martii Jun 23, 2014

jerone commented Jun 23, 2014

cletusc commented Jun 23, 2014

Martii commented Jun 23, 2014

Martii commented Jun 23, 2014

jerone commented Jun 23, 2014

sizzlemctwizzle commented Jun 23, 2014

Martii commented Jun 23, 2014

jerone commented Jun 23, 2014

jerone commented Jun 23, 2014

Martii commented Jun 23, 2014

Add support for only one last @supportURL, fix naming standardization, and such #189

Add support for only one last @supportURL, fix naming standardization, and such #189

Conversation

Martii commented Jun 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martii commented Jun 23, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerone commented Jun 23, 2014

cletusc commented Jun 23, 2014

Martii commented Jun 23, 2014

Martii commented Jun 23, 2014

jerone commented Jun 23, 2014

sizzlemctwizzle commented Jun 23, 2014

Martii commented Jun 23, 2014

jerone commented Jun 23, 2014

jerone commented Jun 23, 2014

Martii commented Jun 23, 2014

Add support for only one last `@supportURL`, fix naming standardization, and such #189

Add support for only one last `@supportURL`, fix naming standardization, and such #189