Closed
Bug 955303
Opened 10 years ago
Closed 10 years ago
Leading whitespace is lost from messages
Categories
(Chat Core :: General, defect)
Chat Core
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.4
People
(Reporter: aleth, Assigned: aleth)
References
Details
Attachments
(1 file, 1 obsolete file)
900 bytes,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1870 at 2013-02-02 16:51:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1870 as attmnt 2219 at 2013-02-02 16:51:00 UTC *** Any leading whitespace in messages is lost during the DOM parser call in http://lxr.instantbird.org/instantbird/source/chat/modules/imContentSink.jsm#335. To avoid this, we can wrap the text to be parsed in a <span> (thanks clokep!) As an alternative to the use of .firstChild in the patch, we could look for the first span node, but this seems clearer.
Attachment #8353982 -
Flags: review?(florian)
Comment 2•10 years ago
|
||
*** Original post on bio 1870 at 2013-02-02 16:59:35 UTC *** Nice idea (and btw, the idea wasn't usable because replacing spaces with unbreakable spaces in code makes it unparsable (in JS at least) in a way that is difficult for users to understand). r- because: - This definitely needs a comment explaining why we are adding that span node - Also, I think it would be nicer to import https://bugzilla.mozilla.org/show_bug.cgi?id=830456 first (I think these patches bitrot each other). Thanks for looking at this annoying issue anyway! :)
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1870 as attmnt 2236 at 2013-02-18 12:18:00 UTC *** The patch blocking this has now landed, so here is a new patch with an added comment.
Attachment #8353999 -
Flags: review?(florian)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8353982 [details] [diff] [review] Patch *** Original change on bio 1870 attmnt 2219 at 2013-02-18 12:18:40 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353982 -
Attachment is obsolete: true
Attachment #8353982 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment 5•10 years ago
|
||
*** Original post on bio 1870 at 2013-02-18 13:07:23 UTC *** Drive-by review: would it make more sense to use a div instead of a span? (My reasoning is simply that div is a block level element, span is an inline element.)
Comment 6•10 years ago
|
||
Comment on attachment 8353999 [details] [diff] [review] Patch *** Original change on bio 1870 attmnt 2236 at 2013-02-18 23:11:50 UTC *** Thanks!!! :-)
Attachment #8353999 -
Flags: review?(florian) → review+
Comment 7•10 years ago
|
||
*** Original post on bio 1870 at 2013-02-18 23:26:46 UTC *** Comment on attachment 8353999 [details] [diff] [review] (bio-attmnt 2236) Patch >+ let div = doc.querySelector("span"); I think the variable name should be changed while you're at it.
Comment 8•10 years ago
|
||
*** Original post on bio 1870 at 2013-02-19 00:01:35 UTC *** (In reply to comment #5) > Comment on attachment 8353999 [details] [diff] [review] (bio-attmnt 2236) [details] > Patch > > >+ let div = doc.querySelector("span"); > > I think the variable name should be changed while you're at it. Good suggestion! I renamed the variable from div to span before pushing: http://hg.instantbird.org/instantbird/rev/dcd23e84ee7a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 1870 at 2013-02-19 10:27:12 UTC *** (In reply to comment #5) > >+ let div = doc.querySelector("span"); > > I think the variable name should be changed while you're at it. :) (Either that or we could have gone with comment #3 and used a div ;) )
You need to log in
before you can comment on or make changes to this bug.
Description
•