Runtime has window.locationbar.visible = true, so content can't detect to show navigation

VERIFIED FIXED in Firefox 16

Status

Firefox Graveyard
Webapp Runtime
P2
normal
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

({dev-doc-complete, regression})

Trunk
Firefox 16
dev-doc-complete, regression
Dependency tree / graph
Bug Flags:
in-moztrap -

Details

(Whiteboard: [qa!])

Attachments

(1 attachment, 1 obsolete attachment)

v2
1.16 KB, patch
myk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
I narrowed down the change that switched visibility from false -> true:

http://hg.mozilla.org/mozilla-central/rev/c8f67f81e8df#l1.17
     1.2 +++ b/webapprt/CommandLineHandler.js
    1.17 -                           null);
    1.18 +                           []);

From bug 747990#c5 we were thinking apps could use locationbar.visibility to detect when to show custom navigation controls, but that won't work right now.
(Assignee)

Comment 1

5 years ago
Created attachment 630698 [details] [diff] [review]
v1

Not sure why locationbar.visibility was false when flags included "all" but arguments was null or why it switched to true when arguments changed from null to []...

But the various toolbar visibility are being set to true because the feature flag "all" includes those toolbars.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #630698 - Flags: review?(myk)
Keywords: regression
Priority: -- → P2
Target Milestone: --- → Firefox 16
Comment on attachment 630698 [details] [diff] [review]
v1

>-                           "chrome,dialog=no,all,resizable",
>+                           "chrome",

Removing "all" seems right, as we should explicitly specify which features we want the window to have.  And this fixes the regression.  But it causes one of its own: the window is no longer resizable (at least on Windows, where I'm testing at the moment).  Seems like "resizable" should still be there, and possibly also dialog=no.
Attachment #630698 - Flags: review?(myk) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 631594 [details] [diff] [review]
v2

Curious about the resizability.. can you resize from the bottom-right corner with the gripper? It seemed to resize fine on Mac for me.

And I'm not sure if dialog=no does anything. The code that I was looking at when investigating why various toolbars were visible true.. that code only had parsing for "yes" and nothing for "no".
Attachment #630698 - Attachment is obsolete: true
Attachment #631594 - Flags: review?(myk)
(In reply to Edward Lee :Mardak from comment #3)
> Created attachment 631594 [details] [diff] [review]
> v2
> 
> Curious about the resizability.. can you resize from the bottom-right corner
> with the gripper? It seemed to resize fine on Mac for me.

Can you double-check this on Windows (only cause especially with WebRT, windows-specific issues are definitely prominent compared to Mac)?
Comment on attachment 631594 [details] [diff] [review]
v2

Sorry for the delay reviewing!  I ran into Windows build issues that took a couple days to resolve.


(In reply to Edward Lee :Mardak from comment #3)
> Curious about the resizability.. can you resize from the bottom-right corner
> with the gripper? It seemed to resize fine on Mac for me.

On Windows (at least on 7) there is a border around the entire window instead of a gripper in the corner, and the window is supposed to be resizable at any point along the border.  With the first patch, it isn't resizable anywhere.

With this second patch, however, it is resizable everywhere along the border, as expected.


> And I'm not sure if dialog=no does anything. The code that I was looking at
> when investigating why various toolbars were visible true.. that code only
> had parsing for "yes" and nothing for "no".

I tried removing it, and that removed the window's minimize and maximize/restore titlebar buttons, so it does seem to have an effect, as <https://developer.mozilla.org/en/DOM/window.open> suggests.


Unfortunately, I can't find docs on what "all" means.  I suppose I could dig through the code.  But I tested out window behavior with this patch, and it behaves the way I expect.  Thus r=myk.
Attachment #631594 - Flags: review?(myk) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/161a63862a70

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/161a63862a70
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [qa+]

Updated

5 years ago
Flags: in-moztrap?(jsmith)
Verified. Changing bug title to reflect change.
Status: RESOLVED → VERIFIED
Summary: Runtime has window.locationbar.visibility = true, so content can't detect to show navigation → Runtime has window.locationbar.visible = true, so content can't detect to show navigation
Whiteboard: [qa+] → [qa!]

Updated

5 years ago
QA Contact: jsmith
Might be worth documenting this somewhere on MDN, as app developers have expressed interest in knowing when they are in a "web app" vs. "browser."
Keywords: dev-doc-needed

Updated

5 years ago
Flags: in-moztrap?(jsmith) → in-moztrap-
I added info about checking window.locationbar.visible in this tutorial here:

https://developer.mozilla.org/en-US/docs/Apps/Tutorials/General/App_code#Navigation
Keywords: dev-doc-needed → dev-doc-complete
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.