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

Selector string when updating a select input is incorrect #769

Closed
clemmy opened this issue Jun 23, 2016 · 4 comments
Closed

Selector string when updating a select input is incorrect #769

clemmy opened this issue Jun 23, 2016 · 4 comments

Comments

@clemmy
Copy link

clemmy commented Jun 23, 2016

The problem is that when using the font-family format

var FontFamilyStyle = new Parchment.Attributor.Style('font', 'font-family', { scope: Parchment.Scope.INLINE });
quill.register(FontFamilyStyle, true);

Then if an option on a select on the toolbar has a value of, say, 'Arial Black, Arial, Helvetica Neue, Helvetica, sans-serif', we would run into an error in the update function.

Uncaught SyntaxError: Failed to execute 'querySelector' on 'Element': 'option[value=""Arial Black", Arial, "Helvetica Neue", Helvetica, sans-serif"]' is not a valid selector.

Notice how there are two double quotes in a row.

Platforms: [Chrome 48, Firefox 46 on Mac 10.11]

Version: [1.0.6.Beta]

@jhchen
Copy link
Member

jhchen commented Jun 24, 2016

Can you post a stack trace?

@ghost
Copy link

ghost commented Jun 29, 2016

I'm hitting this issue as well. Basically, if you have a font-family with a space in it (ex. "Arial Black"), Chrome will encapsulate it with quotes when you ask for the font-family of the HTML element.

let fontFamily = "Arial Black";
node.style['fontFamily']  = fontFamily;
fontFamily = node.style[fontFamily]; // fontFamily is now "Arial Black" (including the quotes)

fontFamily = "'Arial Black'"; // Surrounded with single quotes
node.style['fontFamily']  = fontFamily;
fontFamily = node.style[fontFamily]; // fontFamily is still "Arial Black" (with double quotes instead of single ones)

The error is thrown in update of the Toolbar module:

option = input.querySelector(`option[value="${formats[format]}"]`); // Source
option = input.querySelector('option[value="' + formats[format] + '"]') // Built file/from CDN

When the selector is built, the quotes for the value aren't escaped, so you end up with a bad selector (like the original post). Replacing the double quotes with single ones (which @clemmy did in zenreach#1) should fix it. Just have to make sure that the names with spaces are single-quoted wherever you specify them as well.

Please let me know if you want a PR with the changes.

@jhchen
Copy link
Member

jhchen commented Jul 8, 2016

It seems brittle to just switch to single quotes. Do all browsers add double quotes as opposed to single quotes or leaving them unquoted? If multiple fonts are specified like "Arial Black", Arial, "Helvetica Neue", Helvetica, sans-serif, is the spacing between fonts and commas respected or potentially normalized?

@jhchen jhchen closed this as completed in 2fa8ffb Jul 21, 2016
@scottckr
Copy link

scottckr commented Apr 12, 2019

Was there ever a proper fix done for this? Running into this same issue.

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

No branches or pull requests

3 participants