Closed Bug 1203237 Opened 9 years ago Closed 9 years ago

fixed width/height for panel has broken our overflow detection for RTL locales in the ui-showcase, _might_ also be causing RTL locales to be minimally cropped

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox43 fixed)

RESOLVED FIXED
mozilla43
Iteration:
43.3 - Sep 21
Tracking Status
firefox43 --- fixed

People

(Reporter: dmosedale, Assigned: crafuse)

References

(Depends on 1 open bug)

Details

(Keywords: rtl)

Attachments

(1 file, 2 obsolete files)

When the ui-showcase is in RTL mode, we see scrollbars on most of the panel views.  Setting "display: none;" on the .panel element and then removing it makes the problem go away.  If this problem is in the product, it's very high prio, otherwise it's high prio since it badly impairs our ability to detect RTL changes using the showcase.
Rank: 5
I've just played around with this a bit in Windows. It seems that there's something happening during the switch to rtl mode that's causing the scrollbars.

For instance, if I select one of the affected iframes, and change its dir attribute from rtl to ltr and vice versa, then the scrollbars briefly appear.

Changing either the width or height of the iframe to +20px of what it was, then reducing back to the original value clears the scrollbars.

I also notice the .panel class has overflow:hidden which makes it even more strange. Maybe there's some transition or something kicking in?

Probably the easiest way to test in an RTL build is wait for tomorrow's nightly and download from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central-l10n/
Blocks: 1179193
Target Milestone: mozilla43 → ---
Assignee: nobody → chris.rafuse
Depends on: 1204680
Determined it was a browser bug and created bug in Core Layout here: https://bugzilla.mozilla.org/show_bug.cgi?id=1204680
Attachment #8661005 - Attachment is obsolete: true
Attachment #8661006 - Flags: review?(standard8)
Attachment #8661006 - Flags: review?(dmose)
Temporary work around to remove the scrollbars from the HTML tag inside an iFrame in RTL mode.  Overflow visible is applied to the root body (scoped or narrowed selector) to override common.css in the ui showcase.  This leaves overflow hidden on all iframes' inner body tags and hides the scrollbars.
Keywords: rtl
Comment on attachment 8661006 [details] [diff] [review]
Fix iFrame HTML tag scrollbars appearing in RTL mode

Review of attachment 8661006 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/loop/ui/ui-showcase.css
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +body.is-standalone-room{
> +  /* Override the hidden root body in common.css. Very important otherwise you can't

I like the direction, but I think we don't want to override this for the LTR case, so you probably want at least one other selector as well.
Attachment #8661006 - Flags: review?(standard8)
Attachment #8661006 - Flags: review?(dmose)
Attachment #8661006 - Flags: feedback+
Attachment #8661006 - Attachment is obsolete: true
Comment on attachment 8661437 [details] [diff] [review]
Fix iFrame HTML tag scrollbars appearing in RTL mode

Added id to root html to exclude root html for inner iframe scoping.
Attachment #8661437 - Flags: review?(standard8)
Attachment #8661437 - Flags: review?(dmose)
Comment on attachment 8661437 [details] [diff] [review]
Fix iFrame HTML tag scrollbars appearing in RTL mode

Thanks for the update; the fix looks good!  r=dmose

I've got a few comment readily suggestions; I'll apply them before I land the patch.

>diff --git a/browser/components/loop/ui/ui-showcase.css b/browser/components/loop/ui/ui-showcase.css
>index b7b9cae..9887e63 100644
>--- a/browser/components/loop/ui/ui-showcase.css
>+++ b/browser/components/loop/ui/ui-showcase.css
>@@ -1,23 +1,28 @@
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
>+html[dir="rtl"]:not(.outer-html) {
>+  /* Temporary work around and visual hack -
>+  Scope the root body overflow visible to remove the scroll bars that appear
>+  inside an iFrame. Can remove this rule after core layout bug fix has landed.
>+  See bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1204680
>+  */
>+  overflow: hidden;

Please prefer to use the same style comments (eg 

/**
 *
 */

as elsewhere in the file for improved readability.

>+}
>+
> body {
>-  /* Override the hidden in common.css. Very important otherwise you can't
>+  /* Override the body in common.css. Very important otherwise you can't

This sentence is a bit hard to comprehend if you don't already know what it's talking about.  I'm thinking "override the 'overflow: hidden' in common.css" would be easier, though I'm open to other suggestions.

r=dmose with the above tweaks (which I'm happy to make before checkin unless you beat me to it)
Attachment #8661437 - Flags: review?(standard8)
Attachment #8661437 - Flags: review?(dmose)
Attachment #8661437 - Flags: review+
Summary: fixed width/height for panel may have regressed RTL in a big way → fixed width/height for panel may have caused RTL locales to be cropped by a pixel and broken our overflow detection in the ui-showcase
Summary: fixed width/height for panel may have caused RTL locales to be cropped by a pixel and broken our overflow detection in the ui-showcase → fixed width/height for panel has broken our overflow detection for RTL locales in the ui-showcase, _might_ also be causing RTL locales to be minimally cropped
https://hg.mozilla.org/mozilla-central/rev/f1ffe3642598
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1205368
You need to log in before you can comment on or make changes to this bug.