WRMB: Rendering of text within nested font element is not correct

RESOLVED WORKSFORME

Status

()

Core
HTML: Parser
P1
major
RESOLVED WORKSFORME
17 years ago
8 years ago

People

(Reporter: rubydoo123, Unassigned)

Tracking

({compat})

Trunk
Future
x86
Windows 98
compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bugscape: 8495][fix in hand])

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
This comes from a WRMB in bugscape: 8495

If headings contain a nested font element, the line height is not adjusted 
according to the font. Although the second part of the example is bad html, the 
headings are nested in the font element, the character height for a h6 is 
virtually unreadable.

Please see the bugscape bug for more detail. The original website exhibiting 
this problem is: http://www.caisse-epargne.fr/ASP/modele1.asp
(Reporter)

Comment 1

17 years ago
Created attachment 49817 [details]
testcase demonstarting issues with nested font element
(Reporter)

Comment 2

17 years ago
forgot to add WRMB
Severity: normal → major
Summary: Rendering of text within nested font element is not correct → WRMB: Rendering of text within nested font element is not correct

Comment 3

17 years ago
Over to Alex for initial investigation
Assignee: attinasi → alexsavulov

Comment 4

17 years ago
TESTCASE:

<font ... size="...">
<h...> text </h...>
</font>



The second part of the testcase (attachment 49817 [details]) is a quirks mode issue since
NAV/IE ignore the size= attribute (IE choses to size the font like we do for
size="3"). Unlike NAV, IE uses the size= attribute to set the height of the <h.>
elements. 

Since the size= attribute is DEPRECATED 
( see http://www.w3.org/TR/html4/present/graphics.html#edef-FONT )
i think that we should consider the case when a <font> element has a size
attribute always to be a quirk. 

Comment 5

17 years ago
TESTCASE:

<h...><font ... size="..."></font></h...>
The first part of the testcase (attachment 49817 [details]) is a quirks mode issue too.

Like IE we use to size the font in the same way for different size= values, but
up to the default font size for a header <h.> element we do not adapt the height
of the element to the font size (like already observed). Only when the font
height exceeds the default <h.> element height the height will be adapted.

Comment 6

17 years ago
Created attachment 51765 [details]
sample testcase showing the cases where we act different than IE
(Reporter)

Comment 7

17 years ago
adding bugscape ref and topembed keyword
Keywords: topembed
Whiteboard: [bugscape: 8495]

Updated

17 years ago
Blocks: 64833

Comment 8

17 years ago
As I was trying to solve the bug that originates this bug report:

              <!-- illegal markup -->
<font size=1>
 <h6>HEADER IN FONT TEST</h6>
</font>
              <!-- illegal markup -->

I found out that in the frames tree, there is  a block frame made of a <font>
element what is not allowed. This is beacuse the content does not get corrected.

Marc Attinasi is investigating the problem too.

Hold on....

Comment 9

17 years ago
Interestingly, if the markup was made malformed as well as incorrect, like this:

<font size=1><h6>HEADER IN FONT TEST</h6>

then the Parser will fixup the content model to be sensible, something like:

<font></font><H6><font>HEADER IN FONT TEST</H6></font>

in this case we are fine because the font is inside the heading.

Harish is looking at doing this fixup even when the markup is wellformed
(instead of just when it is malformed).

BTW: the problem is not that we have to make a block frame for the FONT. The
problem is that the FONT block frame surrounding the heading establishes a new
font size, and the contained heading (H6 in this case) has a font size specified
relative to the current font. So, we get a new font-basis of 1 from eh <FONT>
tag, and then the H6 specifies that its font size is 0.67em, and that is
relative to the current font-size-basis of 1, so we get teeny tiny fonts. If the
<FONT> is moved to inside of the heading, then the font will be size 1 - still
small, but at least readable.

Comment 10

17 years ago
changing to assigned

AFAIK harish is already working on a fix.
(see Attinasi's previous comment)
Status: NEW → ASSIGNED

Updated

17 years ago
Target Milestone: --- → mozilla0.9.6
From what I understand Harish is working on extending our handling of residual
style to handle this case. 

reassigning to Harish.
Assignee: alexsavulov → harishd
Status: ASSIGNED → NEW

Comment 12

17 years ago
-> P1
Status: NEW → ASSIGNED
Priority: -- → P1

Comment 13

17 years ago
Basically, residual style kicks in for non wellformed cases. So, I tried
diabling the check for wellformedness so that residual style can kick in
whenever a block level element is contained within an inline element. This
causes inline element's ( FONT ) size to be applied to the content within the
block ( H1 )element. However this contradicts bug 77352!!!. That is, when
residual style kicks in, the following markup 
<FONT SIZE="2pt">one<H1>two</H1></FONT> would become 
<FONT SIZE="10pt">one</FONT><H1><FONT SIZE="2pt" moz-rs-heading=>two</FONT><H1>.
The style rule for _moz-rs-heading should make sure that we inherit the size of
H1 and not the size of * FONT *. It looks like we force this rule for quirks
mode only. So,removing the DOCTYPE ( which is strict ) and disabling the
wellformedness check does seem to fix the problem :-). Therefore, the question
is should we consider this style rule for strict mode too?.

IMO, we should consider disabling the check for wellformedness and let it
regress in strict mode because in strict mode inline line should not contain
block and hence would require the content to be corrected.

Comment 14

17 years ago
Created attachment 53660 [details] [diff] [review]
patch v1.0 [ In order for residual style to kick in diable the check for wellformedness in quirks mode ]

Comment 15

17 years ago
typo [ in my previous comment ]

diable/disable

Comment 16

17 years ago
Note: My patch does not try to fix up strict documents. That is, there will be
no change in behavior for testcases provided by beppe ( id=49817 ) and alexandru
( id=51765 )since the DOCTYPE used is strict and therefore would require content
fix up.
Whiteboard: [bugscape: 8495] → [bugscape: 8495][fix in hand]

Comment 17

17 years ago
Comment on attachment 53660 [details] [diff] [review]
patch v1.0 [ In order for residual style to kick in diable the check for wellformedness in quirks mode ]

sr=attinasi
Attachment #53660 - Flags: superreview+

Updated

17 years ago
Attachment #53660 - Flags: review+

Comment 18

17 years ago
r=alexsavulov

Comment 19

17 years ago
Backed out my fix to this bug since it shot up the reflow numbers & page load
time :-(

Comment 20

17 years ago
removing topembed.
is there a chance for a fix that doesn't cause performance problems?
Keywords: topembed

Comment 21

17 years ago
Marek: The fix that I backed out is the correct fix to the problem but
unfortunately with a heavy price. I can't think of an easy solution to this
problem other than fixing our frame construction code ( which I believe is a
huge task ).

Comment 22

17 years ago
Out of time :-( Moving to 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 23

17 years ago
Mark: I'm not sure what to do with this bug. Should we mark this WONTFIX?

Comment 24

17 years ago
I don't this problem will get addressed any time soon. Moving to 0.9.9.
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Keywords: compat

Comment 25

17 years ago
Fixing this would mean rewriting frame construction code. I don't see it
happening in the near future.

This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another 
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Target Milestone: mozilla0.9.9 → Future
Attachment #53660 - Flags: needs-work+
(Reporter)

Comment 26

16 years ago
removing myself from the cc list
->Parser
Status: ASSIGNED → NEW
Component: Layout → Parser
QA Contact: petersen → moied
Assignee: harishd → nobody
QA Contact: moied → parser
Marking this WFM because both the old and the new parser handle this sanely and the report is too vague to determine if there's a problem left.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.