Skip to content

Fix IE8 translating strings that contain html tags. #131

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

Merged
merged 2 commits into from
Dec 8, 2014

Conversation

maxpeterson
Copy link
Contributor

Related to #51

This checks whether the browser (IE8) modifies (capitalises) html tags (e.g. <span>test</span> > <SPAN>test</SPAN>) and if so it uses the browser to modify (capitalise) any tags in the strings used as the translation keys.

This is to replace #127 that I have closed

@maxpeterson maxpeterson force-pushed the fix-ie8-uppercase-fix branch from 3459305 to c6bbb0f Compare November 24, 2014 14:28
@gabegorelick
Copy link
Collaborator

@rubenv Given the lack of IE testing infrastructure, does it make sense to write a test for this? Or should we just merge this?

@rubenv
Copy link
Owner

rubenv commented Nov 24, 2014

Well, we should at least test the before/after situation on IE8, to make sure it's a valid patch.

@maxpeterson
Copy link
Contributor Author

Any thoughts on this?

@rubenv
Copy link
Owner

rubenv commented Nov 29, 2014

It probably makes sense to do this (though I'm not sure about performance, but well, IE8...).

Need to test this though (and I haven't had time for it). The good news is that we need to support IE8 in one of the projects I'm coordinating and testing for that starts soon, so it will get on the agenda.

As for the code, the only remark I have is that we might want to move that test (isUpperCaseTags) out of the setStrings method: it only needs to happen once anyway.

Good stuff, I don't see why this can't be merged (apart from the move mentioned in the previous paragraph), but I'll need to test it first. Sorry about this taking so long.

@maxpeterson
Copy link
Contributor Author

Thanks @rubenv I will move the isUpperCaseTags out of the setStrings. I appreciate that IE8 is probably not your priority.

@maxpeterson maxpeterson force-pushed the fix-ie8-uppercase-fix branch from c212a68 to 64af4ae Compare November 30, 2014 20:04
@maxpeterson maxpeterson force-pushed the fix-ie8-uppercase-fix branch from 64af4ae to e953d8b Compare November 30, 2014 20:09
@rubenv rubenv modified the milestone: 2.0 Dec 8, 2014
@rubenv rubenv merged commit e953d8b into rubenv:master Dec 8, 2014
@rubenv
Copy link
Owner

rubenv commented Dec 8, 2014

Merged, thanks!

addaleax added a commit to addaleax/angular-gettext that referenced this pull request Feb 11, 2016
(This is related to rubenv#131.) The added test translates
`<input type="submit" value="Hello">`, but this does not work
for me locally (Firefox 44.0, Ubuntu 15.10) since the attributes
get reordered (by the browser, I assume). In rubenv#131, it was
noted that something similar happened with some IE versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants