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

fix(compiler): attribute case when parsing HTML in IE9 #4743

Closed
wants to merge 1 commit into from

Conversation

marclaval
Copy link
Contributor

Investigating a test failure in IE9, I found an interesting issue about parsing attributes.
Consider the following code snippet:

myElem = document.createElement('div');
myElem.innerHTML = '<input type="text" ng-control="min" maxlength="3" ABC="4" efGH="5"></input>';
myElem.outerHTML

Output in Chrome:
<div><input type="text" ng-control="min" maxlength="3" abc="4" efgh="5"></div>
Output in Firefox:
<div><input type="text" ng-control="min" maxlength="3" ABC="4" efGH="5" /></div>
Output in IE9:
<div><input maxLength="3" type="text" ng-control="min" efGH="5" ABC="4"></div>

As you can see, IE9 and Firefox keep the case, and IE9 even modifies it in the particular case of maxlength.
Hence the proposal to lower case all attributes names to harmonize the behaviors.

@marclaval marclaval added action: review The PR is still awaiting reviews from at least one requested reviewer comp: core/view/compiler labels Oct 14, 2015
@marclaval
Copy link
Contributor Author

@IgorMinar Sauce Labs is back to green :)

@@ -45,7 +45,9 @@ function parseText(text: Text, indexInParent: number, parentSourceInfo: string):
function parseAttr(element: Element, parentSourceInfo: string, attrName: string, attrValue: string):
HtmlAttrAst {
// TODO(tbosch): add source row/column source info from parse5 / package:html
return new HtmlAttrAst(attrName, attrValue, `${parentSourceInfo}[${attrName}=${attrValue}]`);
var lowerCaseAttrName = StringWrapper.toLowerCase(attrName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attrName.toLowerCase() should work in both Dart & JS.(we should probably remove the methods (lower & upper) from the Wrapper).

Should you add a comment here ?

@tbosch tbosch added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 15, 2015
@tbosch
Copy link
Contributor

tbosch commented Oct 15, 2015

Looks good

@IgorMinar
Copy link
Contributor

lgtm

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants