#413 ✓resolved
Shawazi

[PATCH][TEST] $$/Selector has issues with "@" signs in class names

Reported by Shawazi | October 28th, 2008 @ 06:07 PM | in 1.7

There is an odd possible glitch. I am using classNames to identify users. To make a long story short - the class can now have an @ symbol in it. If I run this

$$('.sharedUser_name@domain.com')

It will stall the browser and I need to stop the script. I am unsure if the @ symbol is a big no for JS (searching @ doesn't do much in Google) or if this is a prototype issue.

Comments and changes to this ticket

  • John-David Dalton
  • Shawazi

    Shawazi October 28th, 2008 @ 06:54 PM

    I doubt that @ is a valid class name. I don't need it for style itself. I think you're right about the import rule - i recall using @import now that you mention it. I wonder what Prototype is doing when it gets to the @. I would hope they don't try to import ha I read the API and I see that to use $$ they arent accessing the CSS at all, but using JS.

  • John-David Dalton

    John-David Dalton October 28th, 2008 @ 07:02 PM

    • State changed from “new” to “invalid”
  • Andrew Dupont

    Andrew Dupont October 28th, 2008 @ 07:14 PM

    I'd recommend storing that e-mail address in a different attribute. The title attribute, for instance, can hold an arbitrary string. The element can then be selected with $$('*[title="sharedUser_name@domain.com"]').

  • Juriy Zaytsev

    Juriy Zaytsev October 28th, 2008 @ 07:39 PM

    • State changed from “invalid” to “new”

    It looks like "@" is a valid character in class values (as per W3 validator, in HTML 4.01), but not in id values.

  • Shawazi

    Shawazi October 28th, 2008 @ 07:49 PM

    I only used the @ in classes but was using $$ to filter #somediv .badclasswith@. Andrew I like your solution - good idea. I'm going to go for that but the fact that using the @ crashes a browser still makes me think there is a glitch.

  • Juriy Zaytsev

    Juriy Zaytsev October 28th, 2008 @ 07:51 PM

    Should we mark it as a bug?

  • John-David Dalton
  • Shawazi

    Shawazi October 28th, 2008 @ 08:10 PM

    I have no idea how to mark something as a bug since this is my first time here. Figured opening a ticket was doing that. But I do see Andrew Dupont in this dropdown so I figure he's someone important that can tell me what to do ;)

    I forgot about w3 for a minute as well.

  • Juriy Zaytsev

    Juriy Zaytsev October 28th, 2008 @ 08:33 PM

    • Milestone set to 1.7
    • State changed from “new” to “bug”
    • Assigned user set to “Andrew Dupont”

    I'll mark it as a bug.

  • Shawazi

    Shawazi October 28th, 2008 @ 09:20 PM

    Sorry John I somehow missed your last sentence earlier...

    "Which would match .sharedUser_name, the rest fails and gets hung up somewhere else in the code...."

    I used firebug and simply ran the above line and it hung. Therefore it was only one prototype line running so it was not my code making it fail.

    I just ran this test in my application.

    $('resourceContainer').addClassName('blam@wee.com'); $$('#resourceContainer .blam@wee.com');

    If I ran only the second line, nothing bad would happen. If I ran both together, it would hang my browser for more than 10 seconds. I feel like it locked on my other page all together because it had more elements. It also appears to only mess up if that className does exist.

  • Andrew Dupont

    Andrew Dupont October 29th, 2008 @ 12:54 AM

    • Tag set to selector

    We're all in trouble if I qualify as "someone important."

    But, yeah, I'll take this bug. Should be as simple as adding an @ to the class name regex (he said, with ironic foreshadowing...).

  • Juriy Zaytsev

    Juriy Zaytsev October 29th, 2008 @ 01:00 AM

    Looks like a FF bug to me. RegExp.prototype.test seems to freeze poor FF 3.0.3:

    
    /(([\w#:.~>+()\s-]+|\*|\[.*?\])+)\s*(,|$)/.test('#resourceContainer > .blam@wee.com');
    

    (That's a regexp from our Selector.split)

  • Shawazi

    Shawazi October 29th, 2008 @ 02:37 PM

    It appears you're right. It doesn't have that same locking effect in Safari 3 Win. So should I head over to FF forums now? ha

  • Juriy Zaytsev

    Juriy Zaytsev October 29th, 2008 @ 06:22 PM

    Shawazi,

    Yes, it would be great if you could open a ticket with Mozilla guys (about this regexp issue).

    Thanks.

  • Shawazi

    Shawazi October 29th, 2008 @ 08:09 PM

    You can stay posted here:

    https://bugzilla.mozilla.org/sho...

    I just submitted it. It doesn't seem like a critical bug, but their dropdown said that critical means browser hang or crash so I went with that.

  • Juriy Zaytsev

    Juriy Zaytsev October 31st, 2008 @ 12:30 AM

    Another ticket (http://prototype.lighthouseapp.c... seems to be related to this issue (also freezes FF):

    
    /(([\w#:.~>+()\s-]+|\*|\[.*?\])+)\s*(,|$)/.test('.picture-news-tabs li:not(.selected) a|h2');
    
  • Andrew Dupont

    Andrew Dupont February 28th, 2009 @ 07:07 PM

    • Assigned user changed from “Andrew Dupont” to “Christophe Porteneuve”

    Christophe, can you do some regex surgery on this one and find a way to change it so it both (a) includes the @ character and (b) doesn't hang Firefox?

  • Juriy Zaytsev

    Juriy Zaytsev February 28th, 2009 @ 07:35 PM

    We should probably also vote on Mozilla's ticket (I just did). Surprisingly, it is still under "unconfirmed" status.

  • Christophe Porteneuve

    Christophe Porteneuve March 1st, 2009 @ 02:16 AM

    Hey guys,

    I'll try and play around with the regex tomorrow. I also voted on the bug in Mozilla's bugtrack.

  • Christophe Porteneuve

    Christophe Porteneuve March 1st, 2009 @ 03:11 PM

    • Tag changed from selector to patched, selector, tested
    • Title changed from “$$ selector trouble” to “[PATCH][TEST] $$/Selector has issues with "@" signs in class names”

    Hey guys,

    Well, I stole a few moments from my schedule and tried the master on this in FF3.0.6 Mac, and this doesn't hang. So what gives?

    I did patch selector.js so it allows for @ signs in class names (as W3C specs allow pretty much everything in there that doesn't otherwise conflict with CSS selector syntax). I tested it on Mac with FF3.0.6, Safari 3.2.1 and Opera 9.51 (yeah, I'm a bit lagging there), then on separate XP/SP2s Parallels VMs with resp. IE6, IE7/FF and IE8, and it seems to work fine.

    (As a side not, testSelectorWithEmpty b0rks on all IE's, and on IE8 we get a few more breakages, so we need to pay attention to this for the sake of future compat.).

    However, the usage here (emails as classnames) won't work on CSS selection, because of the TLD part: the dot sign is a native selector syntax, so ".user@domain.tld" means "with class user@domain and with class tld"). It works if you start replacing dots by underscores, for instance (which I did in the test fixture and test case).

    Patch against master attached. 'HTH.

  • Diego Perini

    Diego Perini March 2nd, 2009 @ 01:26 AM

    Christophe, I always hate being in contrast over a good work given for free but I have to disagree here, you shouldn't especially allow the "@" character, instead you should allow any "escaped" character (char preceded by a backslash) as the W3C specs explicitly states:

    http://www.w3.org/TR/css3-syntax...

    Doing the opposite will make impossible to specify all the other "escaped" characters that are instead allowed by specs and with time you will find yourself in need to add more and more until you need to include ">", "<" or some brackets and that will start breaking things in regexps.

    I don't pretend to have given a good explication of the problem we face here but hope the above docs will fill the gap.

    Diego

  • Christophe Porteneuve

    Christophe Porteneuve March 2nd, 2009 @ 08:40 AM

    Hey Diego,

    I hadn't had time to check out more extensive docs on the syntax. I had gone to CSS 2.1 which is less than crystal-clear on the question. Going "any-escaped" will probably munch the regexes as it is, so when I tackle this (hopefully very soon), I might limit the scope of this to class names for starters. I'll let you know.

  • Tobie Langel

    Tobie Langel March 2nd, 2009 @ 09:33 AM

    Can we make sure to stay spec compliant here? Wouldn't want to include anything that bites us back later on.

  • Diego Perini

    Diego Perini March 2nd, 2009 @ 04:46 PM

    Tobie, the fact is the OP is stretching to CSS limits, if we agree $$('.sharedUser_name@domain.com') is an edge case we have to take in consideration what Christophe told about the "dot" part of the TLD being part of native selectors syntax.

    So I see no other way to do that other than as specs suggest:

    $$('.sharedUser_name\@domain\.com')

    only this syntax will allow for the complete character set to be used if necessary.

    @Christophe, CSS2.1 and CSS3 basically says the same in this respect but I wouldn't push on this if it creates releases delays, in my view it can wait and go with your patch in the meantime if it doesn't introduce new regressions.

    Taken from http://www.w3.org/TR/CSS21/synda...:

    In CSS, identifiers (including element names, classes, and IDs in selectors) can contain only the characters [a-z0-9] and ISO 10646 characters U+00A1 and higher, plus the hyphen (-) and the underscore (_); they cannot start with a digit, or a hyphen followed by a digit. Identifiers can also contain escaped characters and any ISO 10646 character as a numeric code (see next item). For instance, the identifier "B&W?" may be written as "B&W\?" or "B\26 W\3F".

    Hope it helps.

  • Christophe Porteneuve

    Christophe Porteneuve March 2nd, 2009 @ 06:11 PM

    Diego,

    Hey, so I skipped that part incorrectly. Makes me feel comfortable with pushing the regex towards full spec compliance, hence allowing for properly escaped chars. Will do.

  • Christophe Porteneuve

    Christophe Porteneuve March 3rd, 2009 @ 10:21 PM

    Guys, I think I need pre-veto on this, if any.

    Looking in detail at http://www.w3.org/TR/css3-syntax..., I realize that regexes won't be enough: we need to process the escape syntaxes to turn them into stuff we can USE in our matchers.

    This processing may seriously impact the performance of Selector, not to mention we'd have to check how native implementations, such as querySelector or the less-ideal XPath way, handle such escape-containing identifiers.

    What's the best way to go here, before I put any significant effort into something?

  • Andrew Dupont

    Andrew Dupont March 3rd, 2009 @ 10:38 PM

    If it's that involved, let's punt on it. We're planning to move to Sizzle for the next major release anyway.

  • Christophe Porteneuve
  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    • Tag changed from patched, selector, tested to patched
    • Assigned user cleared.

    [responsible:none bulk edit command]

  • Tobie Langel

    Tobie Langel March 1st, 2010 @ 12:41 AM

    • Tag changed from patched to patched, section:dom
  • Andrew Dupont

    Andrew Dupont October 17th, 2010 @ 09:23 AM

    • State changed from “bug” to “resolved”
    • Importance changed from “” to “”

    Resolved because we're moving to Sizzle.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Attachments

Referenced by

Pages