Treat the innerHTML attribute as the string value

RESOLVED FIXED

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: stas, Assigned: stas)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Assignee

Description

4 years ago
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.
Assignee

Comment 1

4 years ago
Posted 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+
Assignee

Comment 4

4 years ago
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+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 4 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 → ---
Assignee

Comment 8

4 years ago
James, is there a bug tracking the infra issues we ran into after this landed?
Flags: needinfo?(jlal)
Assignee

Comment 9

4 years ago
(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)
Assignee

Comment 12

4 years ago
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)
Assignee

Comment 14

4 years ago
Comment on attachment 8598054 [details] [review]
[gaia] stasm:1142526-innerHTML-value-relanding > mozilla-b2g:master

Carrying over the r+.
Attachment #8598054 - Flags: review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Depends on: 1163825
No longer depends on: 1163825
Assignee

Comment 17

4 years ago
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.