Closed Bug 1142526 Opened 7 years ago Closed 7 years ago

Treat the innerHTML attribute as the string value

Categories

(Firefox OS Graveyard :: Gaia::L10n, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stas, Assigned: stas)

References

Details

Attachments

(3 files)

When bug 994357 lands, we will be able to use HTML in regular translation values.  While bug 1027117 is about removing innerHTML from .properties, we can also improve the security of existing translations by treating innerHTML attributes as the canonical value of the string.
Attached patch PatchSplinter Review
Something like this.  The patch is based on my DOM overlays branch for bug 994357.  Zibi, what say you?
Assignee: nobody → stas
Status: NEW → ASSIGNED
Attachment #8576693 - Flags: feedback?(gandalf)
Comment on attachment 8576693 [details] [diff] [review]
Patch

Review of attachment 8576693 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, that sounds good. We should also file a bug to migrate all uses of innerHTML to normal value, but that's lower priority.

Would it make sense to add a deprecation notice here?
Attachment #8576693 - Flags: feedback?(gandalf) → feedback+
Comment on attachment 8576701 [details] [review]
[gaia] stasm:1142526-innerHTML-value > mozilla-b2g:master

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=2dd503651c08166bc0a6b5b917ff0571fde87ae6

I added the deprecation warning, too.
Attachment #8576701 - Flags: review?(gandalf)
Attachment #8576701 - Flags: review?(gandalf) → review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
https://github.com/mozilla-b2g/gaia/commit/68a2f9c80cab97551a3e555cd15773a683365f68

backed out due to infra issues (I am going to reland)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
James, is there a bug tracking the infra issues we ran into after this landed?
Flags: needinfo?(jlal)
(In reply to Staś Małolepszy :stas from comment #8)
> James, is there a bug tracking the infra issues we ran into after this
> landed?

Hey James, do you have any updates about the infra problems we ran into here?
Clearing NI here things should be good to land (and I believe have now)
Flags: needinfo?(jlal)
James, I meant your comment 7 in which you backed out this patch citing infra issues.  Is this PR good to re-land now?  I believe the problem was one of the buildbots running an old (pre-36) version of Gecko which suffers from bug 1084502.
Flags: needinfo?(jlal)
Comment on attachment 8598054 [details] [review]
[gaia] stasm:1142526-innerHTML-value-relanding > mozilla-b2g:master

Carrying over the r+.
Attachment #8598054 - Flags: review+
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Depends on: 1163825
No longer depends on: 1163825
Removing the old ni? request on :lightsofapollo.
Flags: needinfo?(jlal)
You need to log in before you can comment on or make changes to this bug.