Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create attribute with null value #598

Open
networkimprov opened this issue Sep 12, 2019 · 7 comments
Open

Create attribute with null value #598

networkimprov opened this issue Sep 12, 2019 · 7 comments

Comments

@networkimprov
Copy link

Can extensions generate an empty attribute, e.g. <a href="..." download>...</a>?
If not, could that be enabled?

I tried these:

mdi.renderer.rules.link_open = function(iTokens, iIdx, iOptions, iEnv, iSelf) {
   iTokens[iIdx].attrs.push(['download', null]); // yields download="null"
   //                       ['download']            yields download="undefined"

   return iSelf.renderToken(iTokens, iIdx, iOptions);
};
@puzrin
Copy link
Member

puzrin commented Sep 12, 2019

https://github.com/markdown-it/markdown-it/blob/master/lib/renderer.js#L177

You may override attrs renderer to support empty values.

@networkimprov
Copy link
Author

Understood, thanks.

Could you consider making a null attrs value yield an empty HTML value in 11.x?

@puzrin
Copy link
Member

puzrin commented Sep 12, 2019

That's strange - nobody asked such feature for 5 years. But i have no principal objections.

@rlidwka
Copy link
Member

rlidwka commented Sep 19, 2019

Could you consider making a null attrs value yield an empty HTML value in 11.x?

Alternative suggestion would be to represent empty values as attrs.push(['download', true]).

I think many people would expect attrSet('download', undefined) to unset the existing attribute rather than set it to empty value.

@puzrin
Copy link
Member

puzrin commented Sep 19, 2019

I'd suggest use notation attrSet('download'), it's clear. Also, i'd avoid use attrSet for unsetting and any kind of unsetting in our api at all.

@Lemmingh
Copy link

I don't think any change is required to support boolean attribute

Your "empty attribute" is canonically named "boolean attribute".

The following work:

attrSet("download", "")

attrSet("download", "download")

I'm afraid the discussion here reveals a few potential bugs of markdown-it

markdown-it/lib/token.js

Lines 170 to 181 in 6e2de08

/**
* Token.attrGet(name)
*
* Get the value of attribute `name`, or null if it does not exist.
**/
Token.prototype.attrGet = function attrGet(name) {
var idx = this.attrIndex(name), value = null;
if (idx >= 0) {
value = this.attrs[idx][1];
}
return value;
};

attrGet() uses null to indicate the absence. Thus, both null and undefined should be reserved. But attrSet() and renderAttrs() don't validate it:

markdown-it/lib/token.js

Lines 153 to 167 in 6e2de08

/**
* Token.attrSet(name, value)
*
* Set `name` attribute to `value`. Override old value if exists.
**/
Token.prototype.attrSet = function attrSet(name, value) {
var idx = this.attrIndex(name),
attrData = [ name, value ];
if (idx < 0) {
this.attrPush(attrData);
} else {
this.attrs[idx] = attrData;
}
};

markdown-it/lib/renderer.js

Lines 168 to 185 in 6e2de08

/**
* Renderer.renderAttrs(token) -> String
*
* Render token attributes to string.
**/
Renderer.prototype.renderAttrs = function renderAttrs(token) {
var i, l, result;
if (!token.attrs) { return ''; }
result = '';
for (i = 0, l = token.attrs.length; i < l; i++) {
result += ' ' + escapeHtml(token.attrs[i][0]) + '="' + escapeHtml(token.attrs[i][1]) + '"';
}
return result;
};


Besides, the access control on Token should be reinforced.

At least, the following should be marked as either "internal" or "deprecated", because they allow users to add attributes that may never take effect, or even destroy the attribute records.

  • attrs
  • attrPush()

The following can be replaced by clearer things. You can take JavaScript Map as a reference.

  • attrIndex()

HTML Standard

2.3.2 Boolean attributes:

The presence of a boolean attribute on an element represents the true value, and the absence of the attribute represents the false value.

If the attribute is present, its value must either be the empty string or a value that is an ASCII case-insensitive match for the attribute's canonical name, with no leading or trailing whitespace.

13.2.5.33 Attribute name state:

if there is already an attribute on the token with the exact same name, then this is a duplicate-attribute parse error and the new attribute must be removed from the token.

@puzrin
Copy link
Member

puzrin commented Oct 10, 2021

Besides, the access control on Token should be reinforced.

Guarantees of 100% validate-able html are out of scope. If using .attr* methods, user is expected to understand well what happens. I'd say, writing plugins requires to inspect source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants