Closed Bug 1261612 Opened 4 years ago Closed 4 years ago

White semi-transparent layer in top of iframes with remote=true

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox49 fixed)

RESOLVED FIXED
Tracking Status
firefox49 --- fixed

People

(Reporter: apastor, Assigned: kanru)

References

Details

Attachments

(5 files, 5 obsolete files)

When oop is enabled, the homescreen is shown with a white transparent layer in top of it.
Forgot to mention that happens when we load the iframes in chrome:// urls
That explain the white I have observed when playing with keyboard.
We probably want to inspect all the iframes and elements that are there. I remember running into similar issues in the past with the callscreen and they were due to having certain pieces of the UI hidden with a mix of code and magic CSS selectors. Switching OOP on and off would give different results due to the iframes being handled differently IIRC.
I'll take a look at this.
Assignee: nobody → kchen
https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/dom/ipc/TabChild.cpp#1253-1275

The background is opaque because we make a remote frame opaque if TabChild::IsRootContentDocument() and <iframe mozbrowser> is considered a root content document. We used to allow mozapp iframe inside another mozapp to be transparent but not a browser frame. Now we don't create mozapp iframe anymore maybe we should only allow <iframe mozbrowser> in chrome to have transparent background, maybe via transparent attribute.

See also bug 1175719
Attached file Gaia pull request (obsolete) —
Attachment #8742669 - Flags: review?(bfrancis)
Attachment #8742668 - Flags: review?(bfrancis)
Attachment #8742669 - Attachment is obsolete: true
Attachment #8742669 - Flags: review?(bfrancis)
Kan-Ru, I see a lot of oddities after building debug mulet with your patch :)
Flags: needinfo?(kchen)
Attached image Capture du 2016-04-19 12-46-57.png (obsolete) —
Still on debug mulet, I have a white layer for keyboard. I suspect it's because you only added the tag for the homescreen?
I can confirm we also need the transparent attribute for the window that we set as "inputmethod" :)
Attachment #8742740 - Attachment is obsolete: true
(In reply to Alexandre LISSY :gerard-majax from comment #11)
> Created attachment 8742738 [details]
> Capture du 2016-04-19 12-45-43.png
> 
> Kan-Ru, I see a lot of oddities after building debug mulet with your patch :)

Is it fixed after you apply transparent attribute to inputmethod?
Flags: needinfo?(kchen)
No, this stays as shown in attachment 8742744 [details]
It is also needed for search app otherwise we cannot see the suggestions
Can we just somehow do it for all the mozbrowser frames with chrome urls?
Hmm, I thought I commented this yesterday.
Should I be expecting new patches here, based on the issues gerard-majax is seeing?
Flags: needinfo?(kchen)
(In reply to Olli Pettay [:smaug] from comment #18)
> Hmm, I thought I commented this yesterday.
> Should I be expecting new patches here, based on the issues gerard-majax is
> seeing?

I will investigate the issue. But I'd like to know what do you think about the approach. Consider it as a feedback? instead, thanks!
Flags: needinfo?(kchen)
Updated patch fixed the devtools issue.
Attachment #8742666 - Attachment is obsolete: true
Attachment #8742666 - Flags: review?(bugs)
Attachment #8743737 - Flags: review?(bugs)
Attachment #8742667 - Attachment is obsolete: true
Attachment #8742667 - Flags: review?(bugs)
Attachment #8743738 - Flags: review?(bugs)
Comment on attachment 8743738 [details] [diff] [review]
Part 2. Allow <iframe mozbrowser> in chrome doc to use transparent attribute. v2

>@@ -5163,18 +5163,22 @@ static bool IsTransparentContainerElement(nsPresContext* aPresContext)
>   if (!docShell) {
>     return false;
>   }
> 
>   nsCOMPtr<nsPIDOMWindowOuter> pwin = docShell->GetWindow();
>   if (!pwin)
>     return false;
>   nsCOMPtr<Element> containerElement = pwin->GetFrameElementInternal();
>-  return containerElement &&
>-         containerElement->HasAttr(kNameSpaceID_None, nsGkAtoms::transparent);
>+
>+  TabChild* tab = TabChild::GetFrom(docShell);
>+
>+  return (containerElement &&
>+          containerElement->HasAttr(kNameSpaceID_None, nsGkAtoms::transparent))
>+    || (tab && tab->IsTransparent());
> }

So I'm not quite happy with this, given that TabChild::GetFrom(docShell); returns tabchild even for sub-docshells, not only the
top-level docshell inside TabChild. So the meaning of the method changes here.
Could you possibly check that if we have tabchild, tabchild->GetDocShell returns the docShell we have here?
Or check that docShell is the topmost docshell.
Attachment #8743738 - Flags: review?(bugs) → review-
Attachment #8743737 - Flags: review?(bugs) → review+
Comment on attachment 8742668 [details] [review]
[gaia] kanru:1261612#transparent_homescreen > mozilla-b2g:kanikani

We probably need this flag set on more frames than this, but r+ for this one.
Attachment #8742668 - Flags: review?(bfrancis) → review+
Checks if the presShell is top PreShell from TabChild.
Attachment #8743738 - Attachment is obsolete: true
Attachment #8751626 - Flags: review?(bugs)
Attachment #8751626 - Flags: review?(bugs) → review+
Since this won't affect other products other than B2G, I'm going to land it on m-i.
Keywords: leave-open
Comment on attachment 8742668 [details] [review]
[gaia] kanru:1261612#transparent_homescreen > mozilla-b2g:kanikani

I added search and inputmethod to also having transparent background. If you think this is OK, could you merge it?
Attachment #8742668 - Flags: review+ → review?(bfrancis)
Kan-Ru, do you mind also pushing it to pine? Thanks!
Flags: needinfo?(kchen)
(In reply to Alexandre LISSY :gerard-majax from comment #30)
> Kan-Ru, do you mind also pushing it to pine? Thanks!

I wouldn't mind if it won't make merging m-c to pine harder.
Flags: needinfo?(kchen)
Comment on attachment 8742668 [details] [review]
[gaia] kanru:1261612#transparent_homescreen > mozilla-b2g:kanikani

Gaia patch looks good, thanks! Hopefully there won't be any more window types we need to add this for...
Attachment #8742668 - Flags: review?(bfrancis) → review+
(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #31)
> (In reply to Alexandre LISSY :gerard-majax from comment #30)
> > Kan-Ru, do you mind also pushing it to pine? Thanks!
> 
> I wouldn't mind if it won't make merging m-c to pine harder.

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