Commit 4abee2e1 authored by Sanket Joshi's avatar Sanket Joshi Committed by Commit Bot

Account for viewport scale when computing zoom factor for color popups

This change fixes a bug where the popup for the color picker and color
suggestion picker appears larger than expected on certain device form
factors, leaving a lot of spare whitespace surrounding the picker
inside the popup.

The issue is that the zoom factor used in popup window resizing does
not take into account viewport scale. (Other popups like date do this,
but the color popups do not.)

Bug: 1015301
Change-Id: I74f92cdb1649d531a2964fe7974ca4d8ff4bc0bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866278
Commit-Queue: Sanket Joshi <sajos@microsoft.com>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707154}
parent 0819393e
......@@ -111,7 +111,7 @@ void ColorChooserPopupUIController::WriteColorPickerDocument(
PagePopupClient::AddProperty("selectedColor",
client_->CurrentColor().Serialized(), data);
AddProperty("anchorRectInScreen", anchor_rect_in_screen, data);
AddProperty("zoomFactor", ZoomFactor(), data);
AddProperty("zoomFactor", ScaledZoomFactor(), data);
AddProperty("shouldShowColorSuggestionPicker", false, data);
PagePopupClient::AddString("};\n", data);
data->Append(ChooserResourceLoader::GetPickerCommonJS());
......@@ -151,7 +151,7 @@ void ColorChooserPopupUIController::WriteColorSuggestionPickerDocument(
client_->CurrentColor().Serialized(), data);
}
AddProperty("anchorRectInScreen", anchor_rect_in_screen, data);
AddProperty("zoomFactor", ZoomFactor(), data);
AddProperty("zoomFactor", ScaledZoomFactor(), data);
AddProperty("shouldShowColorSuggestionPicker", true, data);
AddProperty("isFormControlsRefreshEnabled",
RuntimeEnabledFeatures::FormControlsRefreshEnabled(), data);
......@@ -201,6 +201,10 @@ Element& ColorChooserPopupUIController::OwnerElement() {
return client_->OwnerElement();
}
ChromeClient& ColorChooserPopupUIController::GetChromeClient() {
return *chrome_client_;
}
void ColorChooserPopupUIController::OpenPopup() {
DCHECK(!popup_);
popup_ = chrome_client_->OpenPagePopup(this);
......
......@@ -66,6 +66,8 @@ class CORE_EXPORT ColorChooserPopupUIController final
void DidClosePopup() override;
private:
ChromeClient& GetChromeClient() override;
void OpenPopup();
void Dispose();
......
......@@ -139,9 +139,7 @@ void DateTimeChooserImpl::WriteDocument(SharedBuffer* data) {
"window.dialogArguments = {\n",
data);
AddProperty("anchorRectInScreen", parameters_->anchor_rect_in_screen, data);
float scale_factor =
frame_->View()->GetChromeClient()->WindowToViewportScalar(frame_, 1.0f);
AddProperty("zoomFactor", ZoomFactor() / scale_factor, data);
AddProperty("zoomFactor", ScaledZoomFactor(), data);
AddProperty("min",
ValueToDateTimeString(parameters_->minimum, parameters_->type),
data);
......@@ -239,6 +237,10 @@ Element& DateTimeChooserImpl::OwnerElement() {
return client_->OwnerElement();
}
ChromeClient& DateTimeChooserImpl::GetChromeClient() {
return *frame_->View()->GetChromeClient();
}
Locale& DateTimeChooserImpl::GetLocale() {
return *locale_;
}
......
......@@ -38,6 +38,7 @@
namespace blink {
class ChromeClient;
class DateTimeChooserClient;
class LocalFrame;
class PagePopup;
......@@ -65,6 +66,7 @@ class CORE_EXPORT DateTimeChooserImpl final : public DateTimeChooser,
void SetValue(const String&) override;
void CancelPopup() override;
Element& OwnerElement() override;
ChromeClient& GetChromeClient() override;
void DidClosePopup() override;
Member<LocalFrame> frame_;
......
......@@ -475,6 +475,10 @@ Element& InternalPopupMenu::OwnerElement() {
return *owner_element_;
}
ChromeClient& InternalPopupMenu::GetChromeClient() {
return *chrome_client_;
}
Locale& InternalPopupMenu::GetLocale() {
return Locale::DefaultLocale();
}
......
......@@ -54,6 +54,7 @@ class CORE_EXPORT InternalPopupMenu final : public PopupMenu,
void SetValue(const String&) override;
void CancelPopup() override;
Element& OwnerElement() override;
ChromeClient& GetChromeClient() override;
float ZoomFactor() override { return 1.0; }
Locale& GetLocale() override;
void DidClosePopup() override;
......
......@@ -32,6 +32,7 @@
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/page/chrome_client.h"
#include "third_party/blink/renderer/platform/wtf/text/character_names.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
......@@ -45,6 +46,12 @@ float PagePopupClient::ZoomFactor() {
return 1;
}
float PagePopupClient::ScaledZoomFactor() {
float scale_factor = GetChromeClient().WindowToViewportScalar(
OwnerElement().GetDocument().GetFrame(), 1.0f);
return ZoomFactor() / scale_factor;
}
#define addLiteral(literal, data) data->Append(literal, sizeof(literal) - 1)
void PagePopupClient::AddJavaScriptString(const String& str,
......
......@@ -39,6 +39,7 @@
namespace blink {
class ChromeClient;
class Document;
class Element;
class Locale;
......@@ -56,9 +57,16 @@ class CORE_EXPORT PagePopupClient {
virtual void SelectFontsFromOwnerDocument(Document&) = 0;
virtual Element& OwnerElement() = 0;
virtual ChromeClient& GetChromeClient() = 0;
// Returns effective zoom factor of ownerElement, or the page zoom factor if
// the effective zoom factor is not available.
virtual float ZoomFactor();
// Returns the zoom factor, adjusted for the viewport scale.
float ScaledZoomFactor();
// Returns a Locale object associated to the client.
virtual Locale& GetLocale() = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment