Last Comment Bug 762195 - Runtime has window.locationbar.visible = true, so content can't detect to show navigation
: Runtime has window.locationbar.visible = true, so content can't detect to sho...
Status: VERIFIED FIXED
[qa!]
: dev-doc-complete, regression
Product: Firefox Graveyard
Classification: Graveyard
Component: Webapp Runtime (show other bugs)
: Trunk
: All All
: P2 normal
: Firefox 16
Assigned To: Ed Lee :Mardak
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks: 732631 747990
  Show dependency treegraph
 
Reported: 2012-06-06 12:30 PDT by Ed Lee :Mardak
Modified: 2016-03-21 12:39 PDT (History)
8 users (show)
jsmith: in‑moztrap-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (1.16 KB, patch)
2012-06-06 13:28 PDT, Ed Lee :Mardak
myk: review-
Details | Diff | Review
v2 (1.16 KB, patch)
2012-06-08 19:34 PDT, Ed Lee :Mardak
myk: review+
Details | Diff | Review

Description Ed Lee :Mardak 2012-06-06 12:30:44 PDT
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.
Comment 1 Ed Lee :Mardak 2012-06-06 13:28:36 PDT
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.
Comment 2 Myk Melez [:myk] [@mykmelez] 2012-06-08 18:08:34 PDT
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.
Comment 3 Ed Lee :Mardak 2012-06-08 19:34:29 PDT
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".
Comment 4 Jason Smith [:jsmith] 2012-06-08 19:36:09 PDT
(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 5 Myk Melez [:myk] [@mykmelez] 2012-06-12 14:06:59 PDT
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.
Comment 7 Ed Morley [:emorley] 2012-06-13 05:57:46 PDT
https://hg.mozilla.org/mozilla-central/rev/161a63862a70
Comment 8 Jason Smith [:jsmith] 2012-06-24 13:45:20 PDT
Verified. Changing bug title to reflect change.
Comment 9 Jason Smith [:jsmith] 2012-07-06 09:04:39 PDT
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."
Comment 10 Mark Giffin [:markg] 2012-08-08 23:28:08 PDT
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

Note You need to log in before you can comment on or make changes to this bug.