Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(169)

Unified Diff: third_party/WebKit/Source/core/page/CreateWindow.cpp

Issue 2905283003: Remove a bunch of dead code around WindowFeatures (Closed)
Patch Set: Add missing #include Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/page/CreateWindow.cpp
diff --git a/third_party/WebKit/Source/core/page/CreateWindow.cpp b/third_party/WebKit/Source/core/page/CreateWindow.cpp
index d3a42e6f25a79cf4891a8abbea211720a194fb72..9582c2967ad33e8be9910da2a602609f4519dcc2 100644
--- a/third_party/WebKit/Source/core/page/CreateWindow.cpp
+++ b/third_party/WebKit/Source/core/page/CreateWindow.cpp
@@ -29,6 +29,7 @@
#include "bindings/core/v8/ExceptionState.h"
#include "core/dom/Document.h"
#include "core/dom/UserGestureIndicator.h"
+#include "core/events/UIEventWithKeyState.h"
#include "core/frame/FrameClient.h"
#include "core/frame/LocalFrame.h"
#include "core/frame/Settings.h"
@@ -39,14 +40,108 @@
#include "core/page/Page.h"
#include "core/page/WindowFeatures.h"
#include "core/probe/CoreProbes.h"
+#include "platform/KeyboardCodes.h"
#include "platform/loader/fetch/ResourceRequest.h"
#include "platform/weborigin/KURL.h"
#include "platform/weborigin/SecurityOrigin.h"
#include "platform/weborigin/SecurityPolicy.h"
+#include "public/platform/WebInputEvent.h"
+#include "public/platform/WebKeyboardEvent.h"
+#include "public/platform/WebMouseEvent.h"
#include "public/platform/WebURLRequest.h"
namespace blink {
+void UpdatePolicyForEvent(const WebInputEvent* input_event,
kinuko 2017/06/01 06:39:28 nit: put these in anon namespace except for Effect
Nate Chapin 2017/06/01 20:57:11 Done.
+ NavigationPolicy* policy) {
+ if (!input_event)
+ return;
+
+ unsigned short button_number = 0;
+ if (input_event->GetType() == WebInputEvent::kMouseUp) {
+ const WebMouseEvent* mouse_event =
+ static_cast<const WebMouseEvent*>(input_event);
+
+ switch (mouse_event->button) {
+ case WebMouseEvent::Button::kLeft:
+ button_number = 0;
+ break;
+ case WebMouseEvent::Button::kMiddle:
+ button_number = 1;
+ break;
+ case WebMouseEvent::Button::kRight:
+ button_number = 2;
+ break;
+ default:
+ return;
+ }
+ } else if ((WebInputEvent::IsKeyboardEventType(input_event->GetType()) &&
+ static_cast<const WebKeyboardEvent*>(input_event)
+ ->windows_key_code == VKEY_RETURN) ||
+ WebInputEvent::IsGestureEventType(input_event->GetType())) {
+ // Keyboard and gesture events can simulate mouse events.
+ button_number = 0;
+ } else {
+ return;
+ }
+
+ bool ctrl = input_event->GetModifiers() & WebInputEvent::kControlKey;
+ bool shift = input_event->GetModifiers() & WebInputEvent::kShiftKey;
+ bool alt = input_event->GetModifiers() & WebInputEvent::kAltKey;
+ bool meta = input_event->GetModifiers() & WebInputEvent::kMetaKey;
+
+ NavigationPolicy user_policy = *policy;
+ NavigationPolicyFromMouseEvent(button_number, ctrl, shift, alt, meta,
+ &user_policy);
+
+ // When the input event suggests a download, but the navigation was initiated
+ // by script, we should not override it.
+ if (user_policy == kNavigationPolicyDownload &&
+ *policy != kNavigationPolicyIgnore)
+ return;
+
+ // User and app agree that we want a new window; let the app override the
+ // decorations.
+ if (user_policy == kNavigationPolicyNewWindow &&
+ *policy == kNavigationPolicyNewPopup)
+ return;
+ *policy = user_policy;
+}
+
+NavigationPolicy GetNavigationPolicy(const WebInputEvent* current_event,
+ bool toolbar_visible) {
+ // If the window features didn't enable the toolbar, or this window wasn't
+ // created by a user gesture, show as a popup instead of a new tab.
+ //
+ // Note: this previously also checked that menubar, resizable, scrollbar, and
+ // statusbar are enabled too. When no feature string is specified, these
+ // features default to enabled (and the window opens as a new tab). However,
+ // when a feature string is specified, any *unspecified* features default to
+ // disabled, often causing the window to open as a popup instead.
+ //
+ // As specifying menubar, resizable, scrollbar, and statusbar have no effect
+ // on the UI, just ignore them and only consider whether or not the toolbar is
+ // enabled, which matches Firefox's behavior.
+ NavigationPolicy policy = toolbar_visible ? kNavigationPolicyNewForegroundTab
+ : kNavigationPolicyNewPopup;
+ UpdatePolicyForEvent(current_event, &policy);
+ return policy;
+}
+
+NavigationPolicy EffectiveNavigationPolicy(NavigationPolicy policy,
+ const WebInputEvent* current_event,
+ bool toolbar_visible) {
+ if (policy == kNavigationPolicyIgnore)
+ return GetNavigationPolicy(current_event, toolbar_visible);
+ if (policy == kNavigationPolicyNewBackgroundTab &&
+ GetNavigationPolicy(current_event, toolbar_visible) !=
+ kNavigationPolicyNewBackgroundTab &&
+ !UIEventWithKeyState::NewTabModifierSetFromIsolatedWorld()) {
+ return kNavigationPolicyNewForegroundTab;
+ }
+ return policy;
+}
+
static Frame* ReuseExistingWindow(LocalFrame& active_frame,
LocalFrame& lookup_frame,
const AtomicString& frame_name,
@@ -78,6 +173,10 @@ static Frame* CreateNewWindow(LocalFrame& opener_frame,
if (!old_page)
return nullptr;
+ policy = EffectiveNavigationPolicy(
Nate Chapin 2017/05/31 16:55:13 Bonus of calling this: here we only need to do it
+ policy, old_page->GetChromeClient().GetCurrentInputEvent(),
+ features.tool_bar_visible);
+
Page* page = old_page->GetChromeClient().CreateWindow(&opener_frame, request,
features, policy);
if (!page)
@@ -92,7 +191,9 @@ static Frame* CreateNewWindow(LocalFrame& opener_frame,
if (!EqualIgnoringASCIICase(request.FrameName(), "_blank"))
frame.Tree().SetName(request.FrameName());
- page->GetChromeClient().SetWindowFeatures(features);
+ page->SetWindowFeatures(features);
+
+ frame.View()->SetCanHaveScrollbars(features.scrollbars_visible);
Nate Chapin 2017/05/31 16:55:12 This was a side effect of ChromeClientImpl::SetScr
// 'x' and 'y' specify the location of the window, while 'width' and 'height'
// specify the size of the viewport. We can only resize the window, so adjust
@@ -133,7 +234,6 @@ static Frame* CreateWindowHelper(LocalFrame& opener_frame,
const WindowFeatures& features,
NavigationPolicy policy,
bool& created) {
- DCHECK(!features.dialog || request.FrameName().IsEmpty());
DCHECK(request.GetResourceRequest().RequestorOrigin() ||
opener_frame.GetDocument()->Url().IsEmpty());
DCHECK_EQ(request.GetResourceRequest().GetFrameType(),

Powered by Google App Engine
This is Rietveld 408576698