Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The overlay child element is set to width 100% with left/right padding.
#overlay {
box-sizing: border-box;
padding: 2em 40px;
width: 100%;
}
In a content-box box model (default) this will cause the box to appear larger than 100% of its container, as seen in Firefox.
We're missing vendor prefixes for -moz and -webkit.
Proposed resolution
Add vendor-prefixed version of the box-sizing property.
Remaining tasks
Review the patch.
Comments
Comment #1
jessebeach CreditAttribution: jessebeach commentedComment #2
YesCT CreditAttribution: YesCT commentedTo review, this might help: https://drupal.org/contributor-tasks/manual-testing
and I think this needs before and after screenshots in different browsers,
as that contributor task document describes:
Comment #3
Snugug CreditAttribution: Snugug commentedThese two lines should be switched, the -webkit property should come before the -moz property.
Additionally, if we are not supporting Android < 4 in Drupal 8 Core, we can drop the -webkit prefix as per http://caniuse.com/css3-boxsizing
Comment #4
jessebeach CreditAttribution: jessebeach commentedIs there really an ordering concern? I just put them in alphabetical order. Do we really need to reroll this to swap the order? Is there a case you know of where the order matters? Truly, you've got my interest piqued now :)
Android versions 2+ are still fairly popular and one extra line doesn't cost us much. Not that one would be using the overlay on a phone. Maybe a tablet. In any case, it's a byte of prevention.
Comment #5
Snugug CreditAttribution: Snugug commentedI agree that Android 2.3 is still important, just was curious as to if we were still targeting it. We should be, so I'm OK with keeping it.
As for the ordering, it's not a matter of CSS override ordering for this feature, but when it comes to standards, when writing vendor prefixes, the correct order is -webkit, -moz, -ms, -o, standard. This is important for vendor prefixed properties that change when they become standards, but just so happens to not be the case here. That being said, I'd prefer to see us follow best practices when dealing with vendor prefixes even if it's swapping two lines of code.
Comment #6
jessebeach CreditAttribution: jessebeach commentedIncidence of -moz before -webkit in core modules.
/^\s+(\-moz).*\n.*\n?\s+(\-webkit).*$/g
and the reverse.
/^\s+(\-webkit).*\n.*\n?\s+(\-moz).*$/g
-webkit then -moz is slightly more prevalent as a pattern. elk1773, said she wanted to review this. Want to reroll and reverse the order of the properties?
Comment #7
ekl1773Rerolled to reorder vendor prefixes.
Comment #8
jessebeach CreditAttribution: jessebeach commentedIt applies, thanks @ekl1773!
We still need a manual testing review to get this RTBC'd.
Comment #9
ekl1773Alright, I did what I could testing in all the browsers, but I didn't do any mobile testing. It looks like this solves the original problem, but when I tested in IE 10, the overlay slides under the menu to the left. Second opinion on the IE situation?
Comment #10
jessebeach CreditAttribution: jessebeach commentedWe ran into this issue in a bad way during usability tests. The fix is super simple and makes a big difference for end users. Let's please get it in.
I've checked IE9 and IE10 for the overlap issue mentioned by ekl1773 in #9. Changes to the Toolbar CSS since this comment was posted have resolved the overlap issue:
Comment #11
alexpottCommitted 075b602 and pushed to 8.x. Thanks!