Element from comments #1032

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

CrypticSwarm commented Sep 21, 2011

fixes issue 1031

Checks to see if there is a comment before a

CrypticSwarm added some commits Sep 21, 2011

@CrypticSwarm CrypticSwarm test case for comments first for a tr Elements.from e7c80fd
@CrypticSwarm CrypticSwarm fixes issue 1031
Elements.from should check if comment is first when injecting as a table.
7a59ed5

@timwienk timwienk and 1 other commented on an outdated diff Sep 21, 2011

Source/Element/Elements.From.js
@@ -25,11 +25,11 @@ provides: [Elements.from, Elements.From]
Elements.from = function(text, excludeScripts){
if (excludeScripts || excludeScripts == null) text = text.stripScripts();
- var container, match = text.match(/^\s*<(t[dhr]|tbody|tfoot|thead)/i);
+ var container, match = text.match(/^\s*(<!--.*?-->)?\s*<(t[dhr]|tbody|tfoot|thead)/i);
@timwienk

timwienk Sep 21, 2011

Owner

Mind changing this to a non-capturing group?

(?:<!--.*?-->)

@CrypticSwarm

CrypticSwarm Sep 21, 2011

Contributor

Noted.
Do you think it would be better to handle more than one comment?

(?:<!--.*?-->\s*)*

@timwienk timwienk commented on an outdated diff Sep 21, 2011

Source/Element/Elements.From.js
if (match){
container = new Element('table');
- var tag = match[1].toLowerCase();
+ var tag = match[2].toLowerCase();
@timwienk

timwienk Sep 21, 2011

Owner

See previous comment, changing that to a non-capturing group means this can be reverted to match[1].

Contributor

CrypticSwarm commented Apr 1, 2013

Closing this pull request. No activity in 2 years probably means its stale, out of date.

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