Page MenuHomePhabricator

Vector: Both selected and un-selected .vectorTabs should have same defined height value
Closed, ResolvedPublic

Description

Attempting to force a universal change of the box-sizing: attribute throughout (via the following css applied locally)

.mediawiki *, .mediawiki *::after, .mediawiki *::before, .mediawiki {
    -webkit-box-sizing: border-box;
    -moz-box-sizing: border-box;
    box-sizing: border-box;
}

... causes the right-side border images of "un-selected" .vectorTabs to shrink.

Adding the following to the corresponding .less file should correct this problem.

div.vectorTabs span {
    height: 100%;
}

(No adverse side-effects with the addition as far as I can tell.)


Version: Master

Event Timeline

GOIII created this task.Jul 4 2015, 12:41 PM
GOIII raised the priority of this task from to Needs Triage.
GOIII updated the task description. (Show Details)
GOIII added projects: Vector, good first bug.
GOIII added subscribers: GOIII, Jdforrester-WMF, matmarex, Edokter.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 4 2015, 12:41 PM

Where can I see the bug in action?

GOIII added a comment.EditedJul 4 2015, 1:47 PM

@Edokter - Just apply the first .css snippet...

.mediawiki *, .mediawiki *::after, .mediawiki *::before, .mediawiki {
    -webkit-box-sizing: border-box;
    -moz-box-sizing: border-box;
    box-sizing: border-box;
}

... to your local vector.css file, give it a bit and then view any page using the .vectorTabs class.

The border image for un-selected or right side tabs should shrink compared to tabs that are selected (or is the opposite, left side).

Sorry, I am wrong to asume the first snippet is not already in Vector? If not, why put a patch in core to fix a local CSS issue?

GOIII added a comment.Jul 4 2015, 2:59 PM

Adding the snippet simply reveals the inconsistency (i.e. the lack of a height: setting for spans while other elements that are descendants/children of .vectorTabs have one).

The fact that vector was rolled out & then remained box-sizing: "ignorant" for the most part doesn't change the fact its a valid attribute today (see how universal border-box is accomplished in mobile mode for example).

I was just hoping to properly rectify this disparity in case others try to eliminate as much use of content-box as possible too.

A local fix is already in place & trouble free for some time now fwiw.

You may want to reword your proposal, because I somehow still have the idea that you want to apply the first snippet somewhere, and I would certainly oppose that.

GOIII added a comment.EditedJul 4 2015, 5:51 PM

Yes I want to apply the first snippet locally. I'm trying to eliminate all the asinine back and forth taking hold throughout our typical content between the two possible box-sizing: values. Again, this is a rather new .css3/html5 attribute and entities previously created without considering the setting default to one setting in its absence (content-box) while recent additions and the like prefer to collapse the padding, borders, and such (border-box).

Applying the first snippet just also happens to be the means to an end for revealing the lack of the height: setting for unselected (span) elements under the .vectorTabs class. One thing has nothing to do with the other at the end of the day however.

One can also look at this way; Start over & just apply the second snippet by itself locally. Did that "break" anything or change Vector's behavior somehow? Of course not because in the absence of a forced box-sizing: border-box cascade (first snippet), the default reverts automatically to box-sizing: content-box.

Locally as in enwiki? I would also oppose that, as I do see other elements shrinking, such as the search box. So a universal box-sizing:border-box is ill-conceived. There is nothing in the way to choose border-box for certain elements, but it makes no sense whatsoever to apply it to all elements; they have been designed with content-box in mind after all.

GOIII added a comment.Jul 4 2015, 6:28 PM

Tsk... Wikipedia, Wikipedia, Wikipedia. I meant locally as in en.wikisource.

Trying to do that on en.wiki would not be "possible" since there literally hundreds if not thousands of instances where forcing the cascade would reveal far too many rendering differences & deficiencies. I'm talking about correcting less than 2 dozen major "instances" on en.wikisource if that many.

.wikieditor-ui > * for example cannot take border-box as it stands today so the box-sizing attribute is added manually by us with content-box as its value.

And if it made no sense to apply to all elements why is it done in our Mobile Mode? Look around - its being done alright ;)

So if you oppose adding the 2nd snippet to the default Vector .less file no matter how I try to reveal the existing disparity, then I guess there is not much else I can do to convince you.

GOIII added a comment.EditedJul 4 2015, 6:37 PM

Locally as in enwiki? I would also oppose that, as I do see other elements shrinking, such as the search box.

And please... the search box shrank because the "margins" top, left & bottom became relatively equal while the place holder-text & magnifying glass image became vertically aligned to 'middle'. Shift the focus to the search field and you'll see the blinking line-cursor is now spaced equally from the top, left and bottom of the input field's borders too.

The width of the search field remained the same; only extraneous border &/or padding widths where no longer calculated as part of the over-all width (12.6em).

matmarex closed this task as Declined.Jul 6 2015, 5:43 PM
matmarex claimed this task.

Setting box-sizing: border-box will break interfaces and is not supported. Most interfaces are not designed to continue working correctly after someone randomly reduces some widths and heights, and that's basically what setting box-sizing: border-box everywhere does.

If you put that in MediaWiki:Common.css anywhere, the people who inevitably come to debug it will hate you, will probably revert that, and then other on-wiki things which started assuming box-sizing: border-box since you put it there will break, and the other people who come to debug that will also hate you. Please don't do it.

Mobile's setting this everywhere is also a big mistake. They can usually get away with it since they control all of the page (unlike various pieces of MediaWiki and extensions, which each deal only with their own piece of interface), but as soon as they don't things break all over – for example it has caused issues with VisualEditor: T85068. There's a task T86366 asking to remove it.

So yeah. Don't.

(That said, if you want to submit the Vector patch you described just for consistency, I'd be happy to merge it. Just don't even mention box-sizing. ;) )

To recap, using box-sizing: border-box:

  • Is not wrong if:
    • You control everything on the page you're applying it to
    • You're building a website from scratch and think this will be more convenient
    • You need it for particular elements, for example something with width: 100% and border (like the editing textarea)
  • Is wrong if:
    • You do not control everything on the page you're applying it to
    • You try to retroactively apply it on top of already existing CSS
    • You end up needing a blanket content-box override for some large piece of the page
GOIII added a comment.Jul 6 2015, 9:32 PM

(That said, if you want to submit the Vector patch you described just for consistency, I'd be happy to merge it. Just don't even mention box-sizing. ;) )

I only mentioned it initially to provide a means to an end in revealing the discrepancy for those who cared to verify my "assertion". It only became paramount when that nuance became the subject of the report instead of just the means to an end. The fact the shrinkage on the right side border image on unselected .vectorTabs occurs under that condition was not & could not be refuted regardless.

Again, applying the second snippet on its own quietly brings the "lacking" unselected .vectorTabs' background image (e.g. a span element) inline with the 'selected' scenarios. No adverse effect either way.

I'd gladly open another Task if that is what will get the job done; just tell me how to label it and phrase the request. I thought this one was on target at the opening - it just went south unintentionally when the focus became the part about revealing the discrepancy instead of the discrepancy itself. Thanks

GOIII added a comment.Jul 6 2015, 10:07 PM

To recap, using box-sizing: border-box:

  • Is not wrong if:
    • You control everything on the page you're applying it to
    • You're building a website from scratch and think this will be more convenient
    • You need it for particular elements, for example something with width: 100% and border (like the editing textarea)
  • Is wrong if:
    • You do not control everything on the page you're applying it to
    • You try to retroactively apply it on top of already existing CSS
    • You end up needing a blanket content-box override for some large piece of the page

All points well taken here but they deny the practiced reality - nearly every border setting I've ever seen is set in pixels and a good number of padding values are also set using pixels (px = an absolute value = re-sizes and/or zooms poorly). So its not so much about controlling everything or starting from scratch at all -- IMHO its more about the inconsistency in the measurement unit(s) being applied from one element to the next. If "everything" was set strictly in either all absolute or all relative units of measurement, imho, the differences under either box-sizing value would be expected rather than surprising.

And its that 'unexpected surprise' that rubs folks the wrong way but box-sizing is not really the root cause behind such 'behavior'; the inconsistencies in the type of measurement units being applied most like are.

matmarex reopened this task as Open.Jul 7 2015, 12:28 PM
matmarex removed matmarex as the assignee of this task.
matmarex set Security to None.
matmarex removed a subscriber: matmarex.
Jdlrobson triaged this task as Lowest priority.Sep 24 2015, 12:23 AM
Jdlrobson added a subscriber: Jdlrobson.
Isarra moved this task from Backlog to Bugs on the Vector board.Apr 7 2016, 1:14 AM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 27 2016, 4:27 PM
Aklapper added a subscriber: pmiazga.

@pmiazga: Do you plan to mentor this in GCI 2016 (or someone else has agreed to mentor this)? If yes, please register as a mentor. Thanks!

@bmansurov will mentor this task

Pppery claimed this task.Dec 13 2016, 2:20 AM

I'm reproducing a different effect: When box-sizing:border-box is specified, the lines between the tabs jump upwards, but stay the same length. In any case, height: 100% fixes the issue and does not seem to cause any other side effects.

Change 327288 had a related patch set uploaded (by Pppery):
Specify height of tabs

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

Change 327288 merged by jenkins-bot:
Specify height of tabs

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

bmansurov closed this task as Resolved.Dec 15 2016, 5:07 PM