Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further BarProp cleanup #4431

Closed
annevk opened this issue Mar 13, 2019 · 6 comments
Closed

Further BarProp cleanup #4431

annevk opened this issue Mar 13, 2019 · 6 comments
Labels
clarification Standard could be clearer

Comments

@annevk
Copy link
Member

annevk commented Mar 13, 2019

My assumption is that the user interface elements the BarProp talks about can be controlled via window.open(). It would be good to test if they can actually be flipped from true to false somehow and define the underlying model more clearly.

This would be a follow-up to #4430.

@annevk annevk added the clarification Standard could be clearer label Mar 13, 2019
annevk added a commit that referenced this issue Mar 14, 2019
Also describe what happens when the browsing context is discarded.

Tests: web-platform-tests/wpt#15815.

Follow-up: #4431.

Fixes #2579.
@annevk
Copy link
Member Author

annevk commented Mar 12, 2021

@arai-a when working on #5872 did you by any chance also look at these BarProp properties?

@arai-a
Copy link
Contributor

arai-a commented Mar 13, 2021

yes, I checked the properties as well.
(testcase at https://arai-a.github.io/window-open-features/test.html shows those properties)

and yes, there's inconsistency between browsers, about which property becomes false for which condition.

the following is tested on macOS (that may affect "menubar")

Normal window (tab)

Firefox Safari Chrome Edge
locationbar.visible true true true true
menubar.visible true true true true
personalbar.visible true true true true
scrollbars.visible true true true true
statusbar.visible true true true true
toolbar.visible true true true true

Popup (with width=500)

Firefox Safari Chrome Edge
locationbar.visible true false false false
menubar.visible false true false false
personalbar.visible false false false false
scrollbars.visible true true false false
statusbar.visible true true false false
toolbar.visible false false false false

So, the values returned by Firefox and Safari almost matches the visual.

  • personalbar: always hidden
  • scrollbars: becomes visible if necessary
  • statusbar: on Firefox, statusbar is shown as overlay if necessary. (I'm not sure if statusbar exists on Safari)
  • toolbar: always hidden

Differences are:

  • locationbar: shown for Firefox, hidden for Safari (so this is reasonable difference)
  • menubar: in both Firefox and Safari, a popup doesn't have menu, but there's global menu at the top of screen anyway

And Chrome and Edge returns same value for all properties, just depending on whether it's popup or not.
(EDIT: this was not true. Chrome reflects the UI-related feature values, and width=500 makes them false)

Also, personalbar value doesn't reflect the actual visibility, in normal window.
Regardless of showing it or not, it returns true on normal window, and false on popup.

Then, so far, I don't think any window.open feature affects each BarProp independently.

@annevk
Copy link
Member Author

annevk commented Mar 13, 2021

Thanks, I have to say I rather like the model Chrome has, just return false for popups. Not sure we should really tell websites about what UI elements are visible or not.

arai-a added a commit to arai-a/html that referenced this issue Mar 25, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/wpt that referenced this issue Mar 25, 2021
arai-a added a commit to arai-a/html that referenced this issue Mar 26, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/wpt that referenced this issue Apr 20, 2021
arai-a added a commit to arai-a/html that referenced this issue Oct 8, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/html that referenced this issue Oct 12, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/html that referenced this issue Oct 12, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/wpt that referenced this issue Oct 12, 2021
arai-a added a commit to arai-a/html that referenced this issue Oct 13, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
arai-a added a commit to arai-a/wpt that referenced this issue Oct 27, 2021
arai-a added a commit to arai-a/html that referenced this issue Oct 27, 2021
Fixes whatwg#4431

Standardize the value of BarProp attributes, while hiding the actual visibility
from the web content for privacy reasons.
@arai-a
Copy link
Contributor

arai-a commented Oct 28, 2021

The Chromium behavior wasn't correct.
I'll check the behavior again and post the updated summary.

@arai-a
Copy link
Contributor

arai-a commented Oct 28, 2021

on Chromium, BarProp returns UI-related features passed to window.open,
with some indirection:

  • locationbar.visible: toolbar feature or location feature
  • menubar.visible: menubar feature
  • personalbar.visible: toolbar feature or location feature (not personalbar feature)
  • scrollbars.visible: scrollbars feature
  • statusbar.visible: status feature
  • toolbar.visible: toolbar feature or location feature

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/bar_prop.cc;l=46-67;drc=5539ecff898c79b0771340051d62bf81649e448d;bpv=1;bpt=1

bool BarProp::visible() const {
  if (!DomWindow())
    return false;

  const WebWindowFeatures& features =
      DomWindow()->GetFrame()->GetPage()->GetWindowFeatures();
  switch (type_) {
    case kLocationbar:
    case kPersonalbar:
    case kToolbar:
      return features.tool_bar_visible;
    case kMenubar:
      return features.menu_bar_visible;
    case kScrollbars:
      return features.scrollbars_visible;
    case kStatusbar:
      return features.status_bar_visible;
  }

  NOTREACHED();
  return false;
}

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/web/web_window_features.h;l=50-55?q=tool_bar_visible&ss=chromium%2Fchromium%2Fsrc&start=11

struct WebWindowFeatures {
...
  bool menu_bar_visible = true;
  bool status_bar_visible = true;
  // This can be set based on "locationbar" or "toolbar" in a window features
  // string, we don't distinguish between the two.
  bool tool_bar_visible = true;
  bool scrollbars_visible = true;

@domenic
Copy link
Member

domenic commented Oct 28, 2021

Thanks! I suspect that changing the BarProp values to just follow "is popup" is not a major compat risk, but @mfreed7 is the right person to make the call.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 2, 2021
…by window.open, and BarProp values for each case, a=testonly

Automatic update from web-platform-tests
HTML: test BarProp and window.open() feature interactions

For whatwg/html#5872, whatwg/html#4431, and whatwg/html#6530.
--

wpt-commits: 29faceaa68f5cb31798cc17fe6868eaf5fa5fb7f
wpt-pr: 28243
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 3, 2021
…by window.open, and BarProp values for each case, a=testonly

Automatic update from web-platform-tests
HTML: test BarProp and window.open() feature interactions

For whatwg/html#5872, whatwg/html#4431, and whatwg/html#6530.
--

wpt-commits: 29faceaa68f5cb31798cc17fe6868eaf5fa5fb7f
wpt-pr: 28243
dandclark pushed a commit to dandclark/html that referenced this issue Dec 4, 2021
Fixes whatwg#5872 by standardizing an internal "is popup" boolean, and how it is impacted by various window.open() features. Implementations can use this boolean (in addition to other signals) to determine whether an opened window is a popup or not.

Fixes whatwg#4431 by changing the various BarProp visible properties to only reflect this "is popup" boolean, instead of reflecting the actual state of user interface elements, or the passed-in window features.

Adds a "popup" window.open() feature to provide a simpler method of requesting a popup.
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
Fixes whatwg#5872 by standardizing an internal "is popup" boolean, and how it is impacted by various window.open() features. Implementations can use this boolean (in addition to other signals) to determine whether an opened window is a popup or not.

Fixes whatwg#4431 by changing the various BarProp visible properties to only reflect this "is popup" boolean, instead of reflecting the actual state of user interface elements, or the passed-in window features.

Adds a "popup" window.open() feature to provide a simpler method of requesting a popup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer
Development

No branches or pull requests

3 participants