Closed Bug 1153022 Opened 9 years ago Closed 9 years ago

Editing text node trims trailing space

Categories

(DevTools :: Inspector, defect)

defect
Not set
minor

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: jruderman, Assigned: bgrins)

References

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(2 files, 1 obsolete file)

      No description provided.
Thanks for spotting this problem, I found the root cause to be in the InplaceEditor widget we use for editing the textContent:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/inplace-editor.js#791

It trims the value in the input, no matter what.
Some of the users of this widget may want this, but we sure don't want this to happen in the case described here. So perhaps we need a new option for this widget.
Mentor: pbrosset
OS: Mac OS X → All
Hardware: x86_64 → All
I'll fix this
Assignee: nobody → bgrinstead
Mentor: pbrosset
Status: NEW → ASSIGNED
Whiteboard: [devedition-40][difficulty=easy]
Attached patch trim-space.patch (obsolete) — Splinter Review
What do you think?  Just don't trim if in multiline, and adds a test. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d3f822af46e
Attachment #8598977 - Flags: review?(pbrosset)
See Also: → 1046803
Blocks: 1046803
See Also: 1046803
Comment on attachment 8598977 [details] [diff] [review]
trim-space.patch

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

This seems to do the trick, but I'm wondering if we shouldn't instead add a new option to the inplace editor rather than assume that non-multilines want values trimmed.
What if I want a single-line editor that doesn't trim the string?
Attachment #8598977 - Flags: review?(pbrosset)
Attached patch trim-space.patchSplinter Review
Changed this to a trimOutput option (true by default so we don't need to change all of the other places that may have been relying on it trimming).
Attachment #8598977 - Attachment is obsolete: true
Attachment #8599511 - Flags: review?(pbrosset)
Comment on attachment 8599511 [details] [diff] [review]
trim-space.patch

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

Ship It!
Attachment #8599511 - Flags: review?(pbrosset) → review+
Keywords: checkin-needed
remote:   https://hg.mozilla.org/integration/fx-team/rev/cd5b4e44e260
Keywords: checkin-needed
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
https://hg.mozilla.org/mozilla-central/rev/cd5b4e44e260
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.