Fix page height for popup windows

RESOLVED FIXED in 2.1 S3 (29aug)

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

unspecified
2.1 S3 (29aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

4 years ago
Blocks: 1052154
(Assignee)

Updated

4 years ago
Blocks: 1053396
What does this mean?
Flags: needinfo?(kgrandon)
(Assignee)

Comment 2

4 years ago
(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.
Flags: needinfo?(kgrandon)
Blocks: 945259
No longer blocks: 946452
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
Flags: needinfo?(fdjabri)
> 
> 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.
Flags: needinfo?(fdjabri)
I have renamed the bug title accordingly, please let me know if this is wrong.
Blocks: 941182
No longer blocks: 945259
Summary: Implement Rocketbar for popup windows → Hide Rocketbar for popup windows
(Assignee)

Comment 6

4 years ago
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.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Summary: Hide Rocketbar for popup windows → Fix page height for popup windows
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S3 (29aug)
(Assignee)

Comment 7

4 years ago
Created attachment 8478737 [details] [review]
Github pull request

Tim or Etienne - could one of you give this a quick review? Thanks!
Attachment #8478737 - Flags: review?(timdream)
Attachment #8478737 - Flags: review?(etienne)
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.)
Attachment #8478737 - Flags: review?(timdream) → review+
Attachment #8478737 - Flags: review?(etienne)
(Assignee)

Comment 9

4 years ago
Master: https://github.com/mozilla-b2g/gaia/commit/31043c27f7a5156ff4abdcb0c99207f34c5e57d0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.