63987 - Increasing/Decreasing fontsize is behaving weird

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Håkan Waara]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; m18) Gecko/20001229
BuildID:    20001229

Let's say 'y' is the number of times one needs to click the increase-textsize
control in Editor to increase the text to its maximum size.

Then say 'x' is the number times I've been clicking.

If x > y then
 a = x - y
End if

Now, say I want to decrease the textsize again. Then i need to click 'a' times
*first* in order to make the size y-1, y-2 and so on.. got it?

Reproducible: Always
Steps to Reproduce:
1. Open up Editor
2. Write in some text, select it
3. Click the 'a' with an uparrow (means increase text size) say, 20 times
4. Now, try to decrease the textsize again..
5. Notice how you have to click 20-maximumfontsize times first, in order to
actually decrease the textsize.

Actual Results:  I had to click a number of times on the decrease-textsize
control before I could actually change the size.

Expected Results:  I click one time on the decrease-textsize control, and the
textsize decreases one step. Without having to click the number of extraclicks
first, as above.

I know this is a messy bugreport but I couldn't figure out a short and clean way
to tell it. Sorry. But, you know, english isn't my native language.

Thanks.

Comment 1

[Stephan Niemz]

confirming on Linux (current build from CVS source)
and changing Platform/OS to all/all

Comment 2

[Håkan Waara]

I know why this happends. Every time one pushes, for example, the "+ font size
button" it adds a "<BIG>" tag, and every time one pushes the -, it adds a "<SMALL>".

In my opinion, this is a really lame way of handling it. We could just have an
integer, which sets a "<FONT size=i>" in the source.

Any comments?

Comment 3

[rubydoo123]

the use of <big> and <small> is being used until we are using css values for 
format, the <font> element has been deprecated and we do not want to use 
deprecated elements

Comment 4

[rubydoo123]

there are a few possible options to helping improve the use of the big/small 
issue:
1. remove the big/small from the toolbar and incorporate a combobox that uses 
the same terms as in the menu Format|Size
2. when the user adds additional <big> elements, and then chooses to go smaller, 
we could remove the outside most <big> element until all <big> elements are 
removed, and then begin to add <small> if the user chooses to go smaller than 
the default size.
3. allow only as many big/small elements as the current font allows, and then 
repeat step 2 when appropriate.

assigning to jfrancis for review and cc cmanske

Comment 5

[Joe Francis]

If I understand the bug report, the issue is not that we aren't removing "bigs" 
when someone clicks "small".  We *are* removing them.  The bug is that we aren't 
aware of whether a big/small tag has any visible effect, so we let the user 
introduce more of them as desired even if they do nothing.  From an html 
authoring point of view, I don't think this is a bug at all.  If the user wants 
20 nested bigs, let them have them.  Maybe they know the document will later be 
viewed with an app that will actually render 20 bigs larger than 19 bigs, etc.  
At one point in it's development, gecko did that too.  Now it doesn't.  

I suspect that fixing this would involve examining resolved style on the 
selection and disabling big/small as appropriate when maximum/minimum sizes were 
reached in the style system.  Daniel, what do you think?

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

This is a CSS issue, I am Cc:ing pierre and marc.

Also try :

<span style="font-size : 48pt">FOOXXXXXX<big>XXXX</big></span>
or
<span style="font-size : 48pt">FOOXXXXXX<span style="font-size :
larger">XXXX</span></span>

Actually, the behavior described by the reporter is acceptable, in terms of CSS
conformance (see CSS2 section 15.2.4). The UA style rule attached to the element
BIG is :

big { font-size : larger }

This 'larger' value increases the real absolute keyword value of font-size. For
instance from 'medium' to 'large'. The CSS 2 spec explicitely says that "The
user agent may have to extrapolate table values if the numerical value goes
beyond the keywords.". Not "should" or "must".

BTW, there is a difference between browsers' common practice and the HTML 4.0
specification which says "BIG: Renders text in a "large" font.". Not a largeR
font, a large font only. Then nested BIGs should not, normally, increase the
font size more than once. The CSS specification, in its appendix A, adds to
confusion : BIG { font-size : 1.17em } ... Anyway...

Even if our code is conformant to the spec, I understand the reporter's point
and think that we should extend our implementation to allow font sizes beyond
the limits of xx-small and xx-large.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

I have just realized that the editor uses only BIG and SMALL for font size
effects... IMHO, this is a bad idea for HTML presentation, and the reporter is
right ; SIZE attribute on FONT should probably be used instead of BIG and SMALL,
if there is no good reason for the actual choice.

Comment 8

[rubydoo123]

we chose to use big/small because the font element is deprecated and wanted to 
avoid using deprecated elements. when we support css values, then we should 
include font size, until then we are limited in what can be used.

Comment 9

[rubydoo123]

reassigning to Daniel. Daniel, is it possible for you to get a css solution in 
to resolve the font size issue?

Comment 10

[rubydoo123]

removing myself from the cc list

Comment 11

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

patch pending, raising priority.

Comment 12

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch #1

This patch makes our implementation of BIG and SMALL conformant to the HTML
4.01
spec because the spec does not imply any higher or lower limit for the font
size
when increased/decreased by BIG or SMALL.

Comment 13

[rods (gone)]

Comment on attachment 106346 [details] [diff] [review]
patch #1

r=, but comment what the index == 6/0 mean more specifically

Comment 14

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 106346 [details] [diff] [review]
patch #1

Why do you add '-moz-smaller' and '-moz-larger'?  Shouldn't the existing
'larger' and 'smaller' work this way?  See
http://www.w3.org/TR/REC-CSS2/fonts.html#value-def-relative-size

Comment 15

[Kathleen :Brade]

RogerMW--you may want to consider taking this patch for your branch (when it's
super-reviewed and landed on the trunk)

Comment 16

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch #2, in answer to dbaron

Comment 17

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 106364 [details] [diff] [review]
patch #2, in answer to dbaron

Forgot to deal with a few cases in this patch ;  new patch will be available on
monday.

Comment 18

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: testcase for 'font-size: larger'

Comment 19

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 106364 [details] [diff] [review]
patch #2, in answer to dbaron

1.5 and 0.9 don't seem symmetric, but I'd expect them to be.  Also, what does
this patch do when you have 'smaller' on a font size that's off of the *top* of
the scale?

Perhaps a better algorithm would be to scale by the ratio of the nearest pair
of sizes in the table?	Is that easy to implement?

Comment 20

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

> 1.5 and 0.9 don't seem symmetric, but I'd expect them to be.

Right, and that's why I obsoleted the last patch 

> Perhaps a better algorithm would be to scale by the ratio of the nearest pair
> of sizes in the table? Is that easy to implement?

I checked that the ratio is always close to 150% between
x-large and xx-large and always close to 90% between x-small and xx-small. So I
wonder if it's worth doing the query..

Comment 21

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: better test case for 'larger' and 'smaller'

Comment 22

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

changing default component

Comment 23

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch #3

Comment 24

[Chris Nalls]

Marking Topembed+ as part of topembed triage

Comment 25

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 106642 [details] [diff] [review]
patch #3

r=dbaron if you:
 * fix the spelling of "parameter"
 * wrap the comments at less than 80 characters

It might be good at some time in the future to move this stuff (perhaps even
the CalcFontPointSize) into FindNext{Smaller,Larger}FontSize.  After all, this
is the only caller...

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 106642 [details] [diff] [review]
patch #3

sr=bzbarsky, but dbaron is right.  I don't like this knowledge of magic
constants like 0, 6, etc... they should either have names or all this logic
should be encapsulated in a function that just gives the nscoord size of the
next larger or smaller size (if this function needs to take aParentFont->mSize,
so be it).

Comment 27

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

checked in (trunk)

Comment 28

[Madhur Bhatia]

V