Closed Bug 61949 Opened 24 years ago Closed 2 years ago

[MARGIN-C]Indenting text twice adds space on top

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED INACTIVE
Milestone:
mozilla1.4final

People

(Reporter: moz_user, Unassigned)

References

Details

(Keywords: testcase, topembed+, Whiteboard: edt_x3,editorbase+)

Attachments

(1 file, 1 obsolete file)

HTML test case
(deleted), text/html
Details 
From Bugzilla Helper:
User-Agent: Mozilla/4.72 [en] (X11; U; Linux 2.4.0-test10 i686)
BuildID:    2000101015 (M18) and 2000120408

On the second indent, the text appears to move down 1/2 a row (extra padding on
top).

Reproducible: Always
Steps to Reproduce:
1. Open E-Mail, select "New Msg"
2. Type a line of text in the compose area
3. Click on indent-text (move right) several times

Actual Results:  On the second indent, the text appears to move down 1/2 a row
(extra padding on top).  Subsequent indents remain on the "same line".

Expected Results:  For each click, the text should remain on the same line and
indent.
Assignee: ducarroz → beppe
Reassign to editor team
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → INVALID
indent uses blockquote, which is a block element, meaning there is a top margin. 
If that statement is true, the top margin should appear on the FIRST indent.  It
only occurs on the SECOND indent.
actually, I am absolutely correct -- take this code:
<html>
<head>
<title>test</title>
</head>

<body>
<tt>this is some text to see the blockquote text</tt>
<blockquote>first blockquote
	<blockquote>second blockquote</blockquote>
</blockquote>
</body>
</html>

and you will see this:
this is some text to see the blockquote text

	first blockquote

		second blockquote 
Apparently I'm not explaining this very well.  I'm trying to indent the SAME
line by hitting indent several times.  The first time I select indent, the text
moves to the right; it does not gain any top margin.  The second time I select
indent (on the same line), the text moves to the right again but also moves down
(the top margin).  The third time I select indent (on the same line), the text
moves to the right again; it does NOT move down any further.

If I was trying to embed multiple blockquote sections, the top margin makes
sense.  I just don't feel it make sense in the scenario I have tried to explain. 

(Sidenote: Netscape 4.75 behaves a little differently; it only indents and does
not change the paragraph style.)
I think this bug was brushed aside too quickly due to misunderstanding of the
problem.  Reopening it so it can get some more attention.  (sorry...)
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
I really think this is a bug in layout. I don't think top margins are supposed 
to be applied by block elements at the top of the document, even when they are 
nested.

Reassigning to buster.

FYI if you load 

  <body><blockquote><blockquote>text</blockquote></blockquote></body>

in the browser and viewer, it renders with a top margin just like it 
does in composer.
Assignee: beppe → buster
Status: REOPENED → NEW
All adjacent vertical margins on normal-flow blocks should collapse to a single
vertical margin with size max(positive margins) - min(negative margins).  We
don't do exactly that (see bug 50959), but we do something rather close.  Could
you provide an HTML testcase showing the problem?
Summary: Indenting text twice adds space on top → [MARGIN-C]Indenting text twice adds space on top
er, make that max(positive margins) + min (negative margins)
Attached file HTML test case (deleted) — 

    
    

    
  
changing qa assign
QA Contact: esther → pmock

    
    

    
  
I see -- if the blockquote or nested blockquotes (or any block element) are the 
first elements in the document, there should not be a top margin. sorry, I was a 
bit slow in grasping your comment. You are correct, there should not be a top 
margin in that instance.

    
    

    
  
Well, according to CSS the margin should exist, but according to some notions of
backwards compatible HTML, there are places where it shouldn't...

    
    

    
  
the spec says that the first element in a document, if it is a block element, 
should include the top margin? can you point me to the definition of that?

    
    

    
  
CSS2 8.3 and 10.6.  (Note that there is no exception for the first element in a
document.  If there were, it wouldn't be possible to have a margin at the top of
a document.)

    
    

    
  
so, the spec does not account for extracting top margins at all? if the first 
element on a page is a heading, does that mean that we should be including the 
top margin too? or does the spec divide out block elements based on the usage?

    
    

    
  
Build reassigning Buster's bugs to Marc.
Assignee: buster → attinasi
Severity: normal → minor
Target Milestone: --- → Future

    
    

    
  
Is this a dup of bug 99765 ?

    
    

    
  
removing myself from the cc list

    
  
Keywords: oeone

    
    

    
  
*** Bug 99765 has been marked as a duplicate of this bug. ***

    
  
Keywords: topembed

    
    

    
  
Me believes this to be an editor base bug.  Please change prod/comp and dev
owner accordingly.  Thanks
Assignee: attinasi → ducarroz
Severity: minor → normal
OS: Linux → All
Priority: P3 → P2
QA Contact: pmock → beppe
Whiteboard: edt_x3
Target Milestone: Future → ---

    
    

    
  
In section 8.3.1 of the CSS 2 Spec states:
Two or more adjoining vertical margins of block boxes in the normal flow
collapse. The resulting margin width is the maximum of the adjoining margin
widths. In the case of negative margins, the absolute maximum of the negative
adjoining margins is deducted from the maximum of the positive adjoining
margins. If there are no positive margins, the absolute maximum of the negative
adjoining margins is deducted from zero.

I do see the issue that Becki describes and yes it does seem incorrect. However,
upon testing this is what I discovered:

If the text string is the first line of data in the note, on the first indent
the selection is indented and a top margin IS NOT added. The second indent
selection indents the text AND ADDS a top margin. Subsequent indents do not
include extra margin.

If the text string is the second line of text (or later), on the first indent
the selection is indented and a top margin IS ADDED. The second indent selection
indents the text and DOES NOT ADD additional top margin.

So, it seems that we have inconsistent behavior in respect to the position of
the text. 

I think the initial confusion was not understanding that indents wrap the text
within a blockquote and the blockquote style applies a 1em top and bottom
margin. In addition, the inconsistent application of the margin also contributed
to that confusion. As for the processing of the margin, I would expect the 1em
margin to be applied regardless position of the text within the document on the
first indent, subsequent margins due to repeated indents should not be included,
regardless to the position.



    
    

    
  
reassigning to kin and adding joe and kathy
Assignee: ducarroz → kin

    
    

    
  
Fixing bug 33784 rather than using our current hack would at least make the
behavior consistent.  The basic problem is that editor is using quirks mode and
quirks mode is, well, quirky.  In particular, our current emulation of
backwards-compatible margin behavior at the top of body and at both ends of
td/th is done entirely in CSS and applies only to the first/last child (not to
first/last descendants of the first/last child, or to anything whose margin
collapses through the first/last child).  That's bug 33784.
Depends on: 33784

    
    

    
  
reset component
Component: Composition → Layout
Product: MailNews → Browser
Hardware: PC → All

    
    

    
  
This behavior is weird, but at least it is consistent. How difficult would it be
to fix this? What's the tradeoff?

    
    

    
  
Doing a bit of work in a somewhat difficult-to-maintain component, and probably
very slight memory/performance overhead.

    
    

    
  
A very simple workaround for all editing sessions is the following
addition to EditorOverride.css :

   blockquote > blockquote { margin-top: 0px }

Simple, harmless, and easy to remove when bug 33784 is fixed.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch from Daniel Glazman (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  
Here is the patch which Daniel suggested.  Note that it works fine in Composer
but not in the browser (so clicking "Browse" looks slightly different).

    
    

    
  
Discussed in edt.  Plussing, for editor correctness.
Keywords: topembed → topembed+
Whiteboard: edt_x3 → edt_x3,editorbase

    
    

    
  
editorbase+
Whiteboard: edt_x3,editorbase → edt_x3,editorbase+

    
  
Keywords: testcase
Target Milestone: --- → mozilla1.4beta

    
    

    
  
The original complaint in this bug was about using indent in the composer, and
that additional vertical spacing is added to the display.

The comments say, in the old days "blockquote" was used to implement the indent.


However, nowadays it seems that blockquote is no longer used for indent!
When I try the original testcase again with, I see that
  <div style="margin-left: 40px;">2<br>
  </div>
is used instead.
And I don't see any problems with additional vertical spacing added.


Doesn't that mean the original issue is fixed and this bug is WORKSFORME ?


Please explain if you think I have not understood the real issue.


    
    

    
  
kaie--you are correct that this isn't a problem in composer if you are using css
content creation (check your prefs).  However, if you are using html tags for
styles you'll get blockquotes and will see this bug.  I think we should land
Daniel's suggested patch and resolve this bug.

    
    

    
  
nominating bug 
Keywords: nsbeta1

    
    

    
  
The proposal currently is to apply a style attribute to the blockquote that
explicitly sets the margin.  Next action item is to check that this roundtrips
with popular clients.

    
  

        Attachment #114808 -
        Attachment is obsolete: true

    
    

    
  
what's the next step for this one?  should the patch be openned up again to get
reviews?

    
    

    
  
Simon can you take a look at what's needed?
Assignee: kin → smontagu
Target Milestone: mozilla1.4beta → mozilla1.4final

    
    

    
  
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

    
    

    
  
So it seems like there are two choices: *either* we land this patch or something
similar, *or* this is a dup of bug 33784.  Kathy, do we not want the patch for
some reason?  (I just see that you obsoleted the attachment, not why ...)

    
    

    
  
Others felt that this was the wrong fix (the obsoleted patch above).  We
discussed it and people thought we should do the right thing rather than make
the browser and editor display different (since the css file is only for
editor).  Kin and Kevin had a strategy for fixing this but I've forgotten what
it is.
QA Contact: rubydoo123 → layout

    
    

    
  
Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3

    
    

    
  
The bug assignee didn't login in Bugzilla in the last 7 months.

:dholbert, could you have a look please?

For more information, please visit auto_nag documentation.


Assignee: smontagu → nobody
Flags: needinfo?(dholbert)

    
    

    
  
Seems there was a lot of activity 19 years ago, and then this went silent.  It's not clear to me that there's anything to be done at this point.


Our behavior here seems correct as far as I can tell.  Chrome/Safari agree with us on the attached testcase if I add a doctype to render it in standards mode.  If I load the testcase as it currently stands (without a doctype), we render it the same as in standards mode; but Chrome/Safari change their rendering and apparently disregard the margin-top:1em on the blockquote.  Their devtools do show that style as being present (as specified-style from the UA stylesheet and as the actual computed style), but they seem to disregard it.  But if I explicitly add margin-top:1em or margin-block-start:1em to one of the blockquote elements, then they change their behavior and match us.


So they're doing something very-odd in this particular quirky edge case, and given the silence here, I don't know that it would help or that it's particularly useful for us to try to match that odd behavior.


Status: NEW → RESOLVED
Closed: 24 years ago → 2 years ago
Flags: needinfo?(dholbert)
Resolution: --- → INACTIVE

    
    

    
  
(In reply to Daniel Holbert [:dholbert] from comment #43)




Their devtools do show that style as being present (as specified-style from the UA stylesheet and as the actual computed style), but they seem to disregard it.  But if I explicitly add margin-top:1em or margin-block-start:1em to one of the blockquote elements, then they change their behavior and match us.


So they're doing something very-odd in this particular quirky edge case, and given the silence here, I don't know that it would help or that it's particularly useful for us to try to match that odd behavior.




Interestingly, it looks like we have a version of that same quirk, but our version only works for a single layer of blockquote.


Here are some testcases to exercise/compare this quirk, which is most-visible in Firefox in testcase 3 here.  Testcases 1 and 2 are in standards mode, testcases 3 and 4 are the same but in quirks mode. (And the even testcases have 2 nested blockquotes instead of 1.)


(1) All browsers render with margin-top:1em:
data:text/html,<!DOCTYPE html><body style="margin:0"><blockquote>Hello

(2) All browsers render with margin-top:1em:
data:text/html,<!DOCTYPE html><body style="margin:0"><blockquote><blockquote>Hello

(3) All browsers render with margin-top:0:
data:text/html,<body style="margin:0"><blockquote>Hello

(4) Firefox renders with margin-top:1em, other browsers render with margin-top:0:
data:text/html,<body style="margin:0"><blockquote><blockquote>Hello



Firefox renders cases 1, 2, and 4 the same; our behavior on 3 looks like it comes from this quirk.css style rule, which sets the margin-top to 0 on blockquote if it's the first thing in the document:

https://searchfox.org/mozilla-central/rev/1f617334179cf28b4b310d1d116ddbc8ef3348ea/layout/style/res/quirk.css#55-63


(And our devtools do usefully show that being the case, i.e. we show a computed margin-top of 0px on the first blockquote element in cases 3 and 4.)


Presumably Blink/WebKit have a more-magical layout-time implementation of their version of this quirk; and their implementation doesn't get reflected in their devtools' visualization of the computed margin-top; and their implementation apparently also nerfs the margin-top of arbitrarily-many nested blockquote elements at the start of the document, rather than the first one (which is why they differ from us on the attached testcase and on case (4) in my snippet above.)


Anyway; given this is an ancient quirks-mode-specific thing with no activity for 19 years, I think it's fine to leave it alone unless there's actual content which compels us to change behavior and standardize a particular behavior here.  It would be nice to have alignment between browsers one way or another, but I'm not sure it's a common enough pattern to prioritize or spend time/resources on that.



          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: