Closed
Bug 22502
Opened 25 years ago
Closed 25 years ago
"white-space: -moz-pre-wrap; width: 10ch;" doesn't work with variable-width fonts
Categories
(Core :: Layout, defect, P1)
Tracking
()
VERIFIED
FIXED
M14
People
(Reporter: akkzilla, Assigned: akkzilla)
Details
(Keywords: regression, Whiteboard: [PDT+] Have workaround)
Attachments
(1 file)
|
(deleted),
patch
|
Details | Diff | Splinter Review |
Plaintext mail uses style="white-space: -moz-pre-wrap; width: 72ch; ..." to
achieve wrapping. This used to work, but on the tip, plaintext mail and the
plaintext edit window no longer wrap; instead, when you type a long line, a
horizontal scrollbar appears. This makes it hard to compose plaintext email
messages, though the message is still sent correctly wrapped.
|
||
Updated•25 years ago
|
Whiteboard: PDT-
|
|
||
Comment 2•25 years ago
|
||
Marking PDT- because you can still compose and send, and there is a workaround
of using HTML compose
nobody's touched this code in a while, so I don't know what the problem might
be. Setting to M14 (the first milestone in which Kipp's bugs will start getting
fixed.) Set to P1.
Summary: [DOGFOOD] [REGRESSION] -moz-pre-wrap doesn't → [DOGFOOD] [REGRESSION] [BLOCK] -moz-pre-wrap doesn't
|
Assignee | |
Updated•25 years ago
|
Whiteboard: PDT-
|
|
Assignee | |
Comment 4•25 years ago
|
||
Requesting PDT reconsideration. Using html compose isn't an option for people
who need to send plaintext mail (e.g. to people who use other mailers, or to
newsgroups or mailing lists) because of bug 17072. Either 17072 or this one
should be marked dogfood since otherwise there's no way to send plaintext mail.
do we have any idea when this stopped working? that would be very helpful to
know.
|
|
Assignee | |
Updated•25 years ago
|
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Comment 7•25 years ago
|
||
It looks like this has magically gotten fixed again ... seems to be working in
today's build. Yeehaw!
akkana: if you could provide a simple test case, I will add that to the
regression test suite I'm building for block/inline layout. Just attach it to
this bug. That'll help QA verify as well.
Regression test cases must be a url that can be run in viewer (not mozilla, so
no XUL) and that demonstrates the problem without any user intervention.
|
|
Assignee | |
Comment 9•25 years ago
|
||
This is broken again -- the plaintext editor isn't wrapping even though it has
<body style="white-space: -moz-pre-wrap; width: 72ch; ">
Status: RESOLVED → REOPENED
|
||
Comment 10•25 years ago
|
||
severity set to critical. assigning to myself, even though I haven't touched a
line of code that effects this, it's in my area so I'll help hunt it down.
again, we can pinpoint a day where it stopped working, I can scan the list of
checkins for that day and hopefully find the culprit.
Chris, can you help with tracking down when this broke, and also which
platform(s) it is broken on?
Assignee: kipp → buster
Severity: normal → critical
Status: REOPENED → NEW
|
|
Assignee | |
Comment 11•25 years ago
|
||
Correction: it turns out it is wrapping, but at roughly double the number of
characters specified in the style. I see this on Linux and Windows both (my Mac
is still building).
Note also that we're still seeing variable width fonts in plaintext, which might
make wrapping at a fixed column width look a little weird ... there's a
separate bug on that (rods?)
Can't pinpoint when it broke since it's been broken off and on for so long, and
in different ways.
Resolution: FIXED → ---
|
|
||
Comment 14•25 years ago
|
||
changed the summary to reflect what's going on.
measuring the width of the body in characters ("width: 72ch") resolves to a
numeric width in twips. It uses a representative character from the current
font to generate the width in twips. If the font is monospace, you get exactly
what you expect. If the font is variable-width, you get 72 representative
characters in width, not a counting of 72 characters per line (egads, what a
performance dog that would be!)
All that is needed is whenever we are creating a plaintext edit window, add a
style attribute on the body "font: monospace;" (careful, don't mess with single
line text controls who also use -moz-pre-wrap). This should be the
responsibility of the application which knows it's a plain text editor *with
wrapping* to set a flag on the editor to indicate that "font: monospace;" is
needed. I guess another way of saying that is whenever you know to set the
wrapping to "width: 72ch;", you also know that you need "font: monospace;" So I
guess a separate flag really isn't necessary if the app is already responsible
for telling us how many characters we should wrap at (or for telling us to pick
some default number of characters, however that is done today.)
Assignee: buster → akkana
Summary: [DOGFOOD] [REGRESSION] [BLOCK] -moz-pre-wrap doesn't → "white-space: -moz-pre-wrap; width: 10ch;" doesn't work with variable-width fonts
|
|
Assignee | |
Comment 15•25 years ago
|
||
I still don't understand why the style system doesn't do this automatically.
What you're basically saying is that "white-space: -moz-pre-wrap; width=" is
useless unless it's also accompanied by a "font: monospace". If moz-pre-wrap
always relies on monospace, isn't there any way it can set that automatically?
I can put font: monospace into the editor code; but it seems like we're working
around a problem that ought to be fixed at a lower level.
Status: NEW → ASSIGNED
Keywords: regression
|
|
||
Comment 16•25 years ago
|
||
Doing it this way is slightly more flexible. It's possible that someone might
want "width: 10 ch;" for a variable width font and really mean that, though
they'd probably have to understand how we measure "ch" widths. I think the
algorithm is in the CSS spec. Maybe it makes sense for some non-english or
symbolic fonts, I don't know. It's a bit of a stretch, but I'd rather have the
flexibility in the engine in case someone has a need, rather than be overly
restrictive. Let the style system interpret the style it's given, not make
guesses about what styles go together. Who knows what's coming in future revs
of CSS that might effect this? If the style system is pure, we're in a much
better position to support new extensions.
|
|
Assignee | |
Comment 17•25 years ago
|
||
Okay, I guess the flexibility is good, though it would be nice to have a default
so all clients who want this functionality don't have to know that both are
required. We should aim at having good documentation on this eventually.
Now, here's the other problem: I have code to build up a string:
"font: monospace; white-space: -moz-pre-wrap; width: 72ch;"
and I attempt to insert that as a style value on the body, via:
bodyElement->SetAttribute(styleName, styleValue);
(styleName is nsAutoString "style"), then I do a fragment dump of the body node
immediately afterward, and what I get is:
body@0x8705bcc style=white-space: -moz-pre-wrap; width: 72ch; refcount=10<
>
In other words, for some reaon it doesn't work to set font: monospace as part of
the value of a style attribute. (I've tried putting the font: monospace at the
end of the string instead of the beginning, doesn't help.)
Any idea why this might be? Who would know more about this? BTW, I'll attach
the patch that shows this.
|
|
Assignee | |
Comment 18•25 years ago
|
||
|
|
Assignee | |
Comment 19•25 years ago
|
||
Turns out that it should be font-family, not font. With that, it seems to work,
and the code has been reviewed and is ready to check in when the tree opens.
Joe expressed some concern about not being able to use a variable-width font for
his plaintext mail windows (apparently this is a fairly common thing to do on
Mac -- I've heard other people on the newsgroups and in bug reports). We both
think that the problem of the width being recalculated for variable width fonts
(so that it typically wraps at twice the number of characters specified) is
still a bug that should be fixed. But meanwhile, I'll check in this workaround
for M14. The remaining bug (which the summary still describes) probably doesn't
need to be beta1/PDT+ once the workaround is checked in.
Sending back to buster in the hope that he knows who owns the font sizing code
used for wrapping inside layout.
Assignee: akkana → buster
Status: ASSIGNED → NEW
Whiteboard: [PDT+] → [PDT+] Have workaround
|
|
||
Comment 20•25 years ago
|
||
I own the layout code that determines the wrapping width. It's not a font
metrics issue, it would mean a wholesale change in the way we calculate where to
wrap. A lot of work, not for first release when I look at the list of Kipp's
bugs that need to get fixed. So when you check in your fix, go ahead and mark
this an enhancement with a milestone of M20 and a keyword "helpwanted." If
someone else wants to tackle this, I can guide them to the right spot. But I
wouldn't recommend any netscape developer spending time on it, since it isn't on
our beta list.
|
|
Assignee | |
Comment 21•25 years ago
|
||
I've checked in the fix, so we'll always see monospace font. Meanwhile, it
turns out, the bug that was actually reported -- that it didn't behave as Steve
said and in fact wrapped at roughly double the number of letters specified (i.e.
it didn't use a representative character, but something much larger) has
magically gotten fixed. Now with variable-width, it uses a width somewhere
between an m and an n, which seems fairly reasonable. I think this bug can be
closed.
Joe, you might want to give some thought to whether we want a UI in the editor
(or elsewhere) for a pref to use a non-monospace font for plaintext. Perhaps in
one of the editor's built-in style sheets? That's probably a separate bug,
though.
Assignee: buster → akkana
|
|
Assignee | |
Comment 22•25 years ago
|
||
Marking fixed.
Status: NEW → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
|
||
Comment 23•25 years ago
|
||
I would like to verify this fix but not sure what steps I need to do. Akkana,
could you please provide a way to check this ?
|
|
Assignee | |
Comment 24•25 years ago
|
||
See the description at the very beginning of this bug. Bring up plaintext mail
or the plaintext editor window, type a bunch of words, and see if it wraps at
somewhere around 72 columns instead of bringing up a horizontal scrollbar.
|
|
||
Comment 25•25 years ago
|
||
With the Feb 09 th build (2000020908), this problem appears to be fixed.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•