1054556 - Fix page height for popup windows

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Comment 1

[Ben Francis [:benfrancis]]

What does this mean?

Comment 2

[Kevin Grandon :kgrandon]

(In reply to Ben Francis [:benfrancis] from comment #1)
> What does this mean?

Solving this will fix bug 1052154 and bug 1053396. I assume for the future, we should just show rocketbar with a close button here, but maybe it shouldn't be editable.

If there is a way to keep existing functionality, without regressing scrolling - we should do that as we're running out of time.

Comment 3

[Ben Francis [:benfrancis]]

Do we have a UX spec for how Rocketbar should behave with popup windows?

As I understand it, the current desired behaviour is:
* window.open() or <a target="_blank"> called from a browser window should open a new browser window (currently broken, see bug 1054215)
* window.open() from an app should open an overlay (popup) window with a close button and nothing else. There's currently a regression which causes the top of the page to be obscured (bug 1052154) which could just be fixed by adding padding above the iframe?
* window.open(url, '_blank') and <a target="_blank"> called from an app should always open a browser window (since bug 1030433 changed this behaviour)

Once "app scope" is implemented (see bug 972320):
* Calls to window.open() or <a> to a URL which falls within the scope of an app should open in the app (we need a UX spec to define when it should navigate the main app frame and when it should create a popup window)
* Calls to window.open() or <a> from an app to a URL which matches the "open-in-app" prefix in its manifest should open within the app (again, UX spec needed).
* Calls to window.open() or <a> to a URL which doesn't fall within the scope of any app should open a browser window, unless being called from an app where the URL matches an "open-in-app" URL prefix in its manifest.

Once full sheets is implemented:
* window.open() should always create a new sheet
* Perhaps the only time the popup window with the close button should appear is if the "dialog" feature of window.open() is used top opt in to it?

In conclusion, I don't think we need to implement Rocketbar for PopupWindow, do we? We should fix the regression and gradually reduce the number of places PopupWindow is used?

The one edge case I can think of is that if a popup window is created from an app with a collapsed Rocketbar, you'll be able to see the title of the app in the statusbar while the popup window (which could be a completely different website) is being displayed with its own title. What should the behaviour be then?


Note that I have a test app for some of these things which you can load in the browser or install as an app http://people.mozilla.org/~bfrancis/hyperlink

Comment 4

[Francis Djabri [:djabber]]

> 
> The one edge case I can think of is that if a popup window is created from
> an app with a collapsed Rocketbar, you'll be able to see the title of the
> app in the statusbar while the popup window (which could be a completely
> different website) is being displayed with its own title. What should the
> behaviour be then?
> 
I think ideally we should not show the rocket bar when pop-up windows are displayed, since the pop-up window has its own title displayed in the chrome below.

Comment 5

[Ben Francis [:benfrancis]]

I have renamed the bug title accordingly, please let me know if this is wrong.

Comment 6

[Kevin Grandon :kgrandon]

The two dependent bugs are only related to page height, so renaming the bugs. We could still file a polish bug to remove the rocketbar, but I don't think that should block FL - we should definitely get the page height fixed ASAP though as this is causing all kinds of problems.

Comment 7

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Tim or Etienne - could one of you give this a quick review? Thanks!

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8478737 [details] [review]
Github pull request

This can land for the code perspective, if that's what you want (before FL and polish etc.)

Comment 9

[Kevin Grandon :kgrandon]

Master: https://github.com/mozilla-b2g/gaia/commit/31043c27f7a5156ff4abdcb0c99207f34c5e57d0