Page MenuHomePhabricator

Remove overflow:hidden for headings
Closed, DeclinedPublic

Description

Since 140a3b84cc8fea6861ca70b8451d399de2149d3d (T28449) the headers have overflow: hidden:

h1, h2, h3, h4, h5, h6 { overflow: hidden }

This was added because of two reasons:

  1. The edit section buttons were on the right margin and they floated away when there are floating elements at the right side.
  2. The lines under h1 and h2 have always full width even when there is a floating object and the lines are visible though transparent floating objects.

The overflow: hidden has some disadvantages:

  • Very large letters that overflows out of the box gets cut. See T37430.
  • Long words that are wider than the space beside of a floating element get cut instead of putting under the floating element.
  • The headers do not float around floating objects. The text is in a rectangle.
  • It prohibits further development like T18691.

Edit section buttons are no floating elements anymore. This reason is gone.

Header border bottoms that are visible though transparent floating elements can still exist. Test case:

{| class="floatright"
|-
! Header text !! Header text !! Header text
|- | Example || Example || Example
|- | Example || Example || Example
|- | Example || Example || Example
|}

Text before the header.

== Header ==

Text after the header.

Maybe there is an other solution to avoid the overlapping that do not have this disadvantages.

Event Timeline

Fomafix claimed this task.
Fomafix raised the priority of this task from to Needs Triage.
Fomafix updated the task description. (Show Details)
Fomafix subscribed.
gerritbot subscribed.

Change 187924 had a related patch set uploaded (by Gerrit Patch Uploader):
Replace overflow:hidden by display:flex for headings

https://gerrit.wikimedia.org/r/187924

Patch-For-Review

Mediawiki still supports IE8, AFAIK. How does the display:flex solution play with older IE browsers?

Change 187926 had a related patch set uploaded (by Gerrit Patch Uploader):
Remove .firstHeading { overflow: visible }

https://gerrit.wikimedia.org/r/187926

Patch-For-Review

How does the display:flex solution play with older IE browsers?

In IE9 and lower does not support display: flex and show it like display: block. For these browser the border button of h2 is visible trough transparent floating objects. I think this is acceptable.

display: flex; is ment to flow multiple boxes next to, or on top of eachother. Only applying it to the header is a rather unpredictable hack, and so it is misapplied here.

@Edokter Do you have any other suggestion?

Fomafix renamed this task from Replace overflow:hidden by display:flex for headings to Remove overflow:hidden for headings.Feb 1 2015, 9:51 AM
Fomafix set Security to None.

@Edokter Do you have any other suggestion?

No, I don't. And frankly, I'm not convinced at all that this is needed; the hidden overflow has always worked with nearby floating elements, and removing it will cause more problems then it solves (as you have demonstrated yourself with the table example).

An other solution to avoid an overflowing border bottom of the headers without adding overflow:hidden to h2 is generating the border bottom by a pseudo element with overflow:hidden:

h1::after,
h2::after {
	border-bottom: 1px solid #aaa;
	content: "";
	display: block;
	overflow: hidden;
}

@Fomafix, that seems like a plausible solution. Consider replacing ::after with :after though; it has better support.

Important hint: This border bottom can not removed by

h2 { border-bottom: none }

It has to checked if somewhere the line is removed on this way.

:after also lacks IE8 support.

I'll note that the idicators already sets overflow: visible hor .firstHeading, so no need for change there. Ans 'resetting' the display to block does nothing.

IE 8 does support :after, it just doesn't support ::after.
http://caniuse.com/#search=pseudo-elements

IE8 supports :after but not ::after. (https://developer.mozilla.org/en-US/docs/Web/CSS/::after#Browser_compatibility)

The skins resets to the default .firstHeading { overflow:visible }. This is not necessary when h1 { overflow: hidden } is removed. When h1 { display: flex } is set a reset to .firstHeading { display:block } in the skin is necessary.

I was confused with last-child.

But display:flex is still misapplied; the flexbox model is meant to distribute available space in a parent container (the one with display:flex) between child containers using an array of other flexbox CSS properties. Only using display:flex makes no sense here, and ends up behaving like display:block anyway.

Have you done some research on the flexbox CSS module? If not, start here: http://css-tricks.com/snippets/css/a-guide-to-flexbox/.

Change 187963 had a related patch set uploaded (by Gerrit Patch Uploader):
Remove overflow:hidden for headings

https://gerrit.wikimedia.org/r/187963

Patch-For-Review

https://gerrit.wikimedia.org/r/187963 has improvements because headings can float around floating objects. Test case:

<div style="clear:both"></div>
<div style="float:right; margin: 0 0 2em 2em; border: 1px solid red; height: 3em; width: 10em;">box</div>
== Line 1<br/>Line 2<br/>Line 3<br/>Line 4 ==

<div style="clear:both"></div>
<div style="float:left; margin: 0 2em 2em 0; border: 1px solid red; height: 3em; width: 10em;">box</div>
== Line 1<br/>Line 2<br/>Line 3<br/>Line 4 ==

IE7 and lower don't support :after so these browser don't get a line below the headings anymore. To support these browsers the following can added:

/* IE7 and lower */
h1,
h2 {
	*border-bottom: 1px solid #aaa;
}

For removing of the line also separate definitions are necessary.

This is an awfull lot of hacks just to add an anchor link. Can we put this aside for now and centralize discussion at T18691?

@Fomafix, https://www.mediawiki.org/wiki/Compatibility#Browser_support_matrix indicates that support below IE8 is not required.

@Edokter, this would mean that only the ::after pseudo element would be a hack. We should certainly centralize discussion at the T18691, but personally just the pseudo element doesn't seem too hacky.

I extracted this discussion from T18691 because it is not related to that feature.

h2 { overflow: hidden } is also a hack. We don't want to cut very long letters. And we don't want a heading that don't flow around floating object. We just want a line under the heading that stops at the margin of a floating object.

https://www.mediawiki.org/wiki/Compatibility#Grade_C requires "[...] content is presented in a readable manner [...]". I think the lines below of the headings are not required to be readable. So it is acceptable to drop the lines for Internet Explorer 7 and lower.

overflow:hidden is perfectly valid CSS and not a 'hack' in any way; it does exactly what it is intended to do. It is the various code proposed here that are hacks because they try to negate lazy coding; if T18691 is not related, then why are we having this discussion about replacing code that works on all browsers with code that does not work on all browsers?

h1:after,
h2:after {
	border-bottom: 1px solid #aaa;
	content: "";
	display: block;
	overflow: hidden;
}

is also perfectly valid CSS and supported by all grade A browsers.

h1,
h2 {
	border-bottom: 1px solid #aaa;
	overflow: hidden;
}

has some unintended disadvantages: It cuts large letters and is does not float around floating objects. It puts everything into a rectangle which is not intended.

It is valid, but overcomplicated and not universally supported; grade A is not all browsers. The hidden overflow does flow around floating objects; I have no idea how you could state otherwise... it was designed to flow around floating objects.

If letters are cut off, the line-height should be adjusted, period. What is the major objection to add anchors without breaking ten other things???

Look at the test case

<div style="clear:both"></div>
<div style="float:right; margin: 0 0 2em 2em; border: 1px solid red; height: 3em; width: 10em;">box</div>
== Line 1<br/>Line 2<br/>Line 3<br/>Line 4 ==

<div style="clear:both"></div>
<div style="float:left; margin: 0 2em 2em 0; border: 1px solid red; height: 3em; width: 10em;">box</div>
== Line 1<br/>Line 2<br/>Line 3<br/>Line 4 ==

with and without https://gerrit.wikimedia.org/r/187963 to see the difference about floating.

It is possible to add a support for Grade C browsers with a separate definition based on the current definition. But I think this is not necessary.

OK, it flows around. But the underline crosses the float; the reason hidden exists in the first place. And with other aternatives now investigated, there is no need to remove it.

The underline does not enter margin of the floating element.

Here is an test case with more text:

<div style="float:right; margin: 0 0 1em 1em; border: 1px solid red; height: 15em; width: 30em;">box</div>
Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat. Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

== Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua ==

Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat. Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

The underline does not enter margin of the floating element.

Wrong.

underline-overflow.png (292×1 px, 20 KB)

I request that this be closed now. I have amply demonstrated regression, and the initial need for this has already been negated.

Which browser did you use to make this screen shot?

@Fomafix, it does look like replacing overflow: hidden with something else will cause several problems and complications for not too much gain. Some other solution will have to be devised for the letters getting cut. As for the clickable anchors, there do seem to be other solutions that might not cause as much complication as this.

@Fomafix, I used Chrome 40, but effect is seen in any browser.

@Edoktor It seams you didn't applied https://gerrit.wikimedia.org/r/187963 and you just removed overflow:hidden.

Here is a screen shot with Chrome 40 and https://gerrit.wikimedia.org/r/187963 with the test case

<div style="float:right; margin: 0 0 1em 1em; border: 1px solid green; height: 30em; width: 10em;">green box</div>
<div style="float:right; margin: 0 0 1em 1em; border: 1px solid red; height: 15em; width: 20em;">red box</div>
Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat. Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

== Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua ==

Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat. Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

T88220.png (432×880 px, 38 KB)

We are exploring different solutions for the anchors and cut-off characters are fixed by a simple line-height fix.

So, why is this needed again?

See task description.

@Edokter Did you created your screen shot with https://gerrit.wikimedia.org/r/187963 or just by removing overflow: hidden?

Yes I did. But AGAIN, I fail to see the benefit; all issues you describe can be solved by other means.

Please check your test environment. You should get the view described in https://phabricator.wikimedia.org/T88220#1008165

How do you want to solve cut letters? You want to increase line-height? To what value?

How do you want to allow a heading a normal flowing around floating objects?

  • The cut-off characters should be solved with line-height. Value depends on font used.
  • A header is typically a single line, so needs no flowing.
  • The anchor (T18691) no longer needs visible.

So, final comment from me here: This is a non-solution that will only cause more problems, which in turn need other fixes with degraded browser support. This change is not only non-essential; it is detrimental.

Can someone please close this?

I created https://gerrit.wikimedia.org/r/187963 PS2 which uses the current implementation as fallback for grade C browsers which don't support :after.

Aklapper triaged this task as Medium priority.Feb 2 2015, 5:01 PM

Here an example where a long word in the heading gets cut (https://de.wikipedia.org/w/index.php?oldid=138351121#Erschlie.C3.9Fung_des_Ortes):

T88220-b.png (600×800 px, 238 KB)

The screen has a size of 800x600.

That is an edge case where a header is caught between two floating images. Again, not worth the host of other regressions that will emerge, should overflow:hidden be removed.

Will you PLEASE stop puching that patch? It is a non-solution to a non-problem, only creating more problems that need fixing, and resurrecting even more problems that were already fixed! Since it is no longer needed anyway, I'm going to ask that a reviewer please -2 that patch.

Issue 2, "long words [...] get cut", can be fixed with word-wrap: break-word. Did you tried that?

Issue 3, "the text is in a rectangle", is a feature and not a bug, in my humble opinion.

Issue 2, "long words [...] get cut", can be fixed with word-wrap: break-word. Did you tried that?

The normal flow is to put to long word under the floating object. overflow: hidden cuts them. word-wrap: break-word breaks them. Somewhere. Without a hyphen at the end. A better workaround would be to simulate the floating under the floating object by min-width: -moz-min-content;. But only for Firefox.

Issue 3, "the text is in a rectangle", is a feature and not a bug, in my humble opinion.

This is point of view. The normal flow is to float around floating objects. I think it is strange that headers not float around and create white areas.

@Edokter Which regressions do you think of?

The normal flow is to put to long word under the floating object.

Which sometimes creates huge amounts of whitespace before the headline because the whole headline gets pushed under the floating object. What's worse? A word cut and unreadable (overflow: hidden), a word broken at an odd place (hidden + break-word) or a headline pushed down, leaving white areas (overflow: visible)?

This is point of view.

Yes, it is. So is yours.

The normal flow is to put to long word under the floating object.

Which sometimes creates huge amounts of whitespace before the headline because the whole headline gets pushed under the floating object. What's worse? A word cut and unreadable (overflow: hidden), a word broken at an odd place (hidden + break-word) or a headline pushed down, leaving white areas (overflow: visible)?

That is inconsistent: The long word in the header gets cut or broken and the long word in the paragraph flows under the floating object as default.

This is point of view.

Yes, it is. So is yours.

Putting the header into a rectangle and cutting words was not the intension of 140a3b84. When you think this is a feature than this is your point of view.

Please abandon any associated patches.

Change 187926 abandoned by Krinkle:
Replace overflow: visible by display: flex

Reason:
Closing per T88220.

https://gerrit.wikimedia.org/r/187926

Change 187924 abandoned by Thiemo Kreuz (WMDE):
Replace overflow:hidden by display:flex for headings

Reason:
Multiple -1, no action for 5 years, not mergeable any more anyway.

https://gerrit.wikimedia.org/r/187924

Change 187963 abandoned by Thiemo Kreuz (WMDE):
Remove overflow:hidden for headings

Reason:
The linked ticket T88220 got declined years ago.

https://gerrit.wikimedia.org/r/187963