Title: [259198] trunk/Source/WebKit
- Revision
- 259198
- Author
- [email protected]
- Date
- 2020-03-30 06:56:32 -0700 (Mon, 30 Mar 2020)
Log Message
[macOS] Datalist dropdown suggestions table can be scrolled too far
https://bugs.webkit.org/show_bug.cgi?id=209721
Reviewed by Tim Horton.
WKDataListSuggestionTableView's enclosing scroll view is set to an incorrect size while laying out the table
view, due to AppKit logic that attempts to adjust the size of the table view's enclosing scroll view after we've
already attempted to set the frame of the scroll view under the overridden call to -layout.
Fix this by refactoring the logic around the suggestion table view's enclosing scroll view; currently,
WKDataListSuggestionTableView overrides -enclosingScrollView to return its own NSScrollView, which it sizes
underneath an overridden call to -layout. This is a bit strange, since the table view is actually a subview of
the scroll view it owns; the fact that laying out the table view causes an ancestor view to change size (and
subsequently invalidate the table view's layout) seems to be what breaks AppKit's enclosing scroll view
adjustment logic.
Instead, we can have own both the table view and the table view's enclosing scroll view.
* UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:
(-[WKDataListSuggestionTableView initWithElementRect:]):
(-[WKDataListSuggestionsController initWithInformation:inView:]):
Move the scroll view initialization logic out of -[WKDataListSuggestionTableView initWithElementRect:] and into
this method. This also means that we can initialize the scroll view with the bounds of the window's content
view, instead of waiting until we lay out the table view underneath the scroll view.
(-[WKDataListSuggestionsController updateWithInformation:]):
(-[WKDataListSuggestionsController invalidate]):
Clear out and remove _scrollView as well when we tear down the controller.
(-[WKDataListSuggestionsController showSuggestionsDropdown:]):
(-[WKDataListSuggestionTableView layout]): Deleted.
(-[WKDataListSuggestionTableView enclosingScrollView]): Deleted.
(-[WKDataListSuggestionTableView removeFromSuperviewWithoutNeedingDisplay]): Deleted.
Now that the controller owns the scroll view, we don't need to override these anymore.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (259197 => 259198)
--- trunk/Source/WebKit/ChangeLog 2020-03-30 13:49:44 UTC (rev 259197)
+++ trunk/Source/WebKit/ChangeLog 2020-03-30 13:56:32 UTC (rev 259198)
@@ -1,3 +1,43 @@
+2020-03-30 Wenson Hsieh <[email protected]>
+
+ [macOS] Datalist dropdown suggestions table can be scrolled too far
+ https://bugs.webkit.org/show_bug.cgi?id=209721
+
+ Reviewed by Tim Horton.
+
+ WKDataListSuggestionTableView's enclosing scroll view is set to an incorrect size while laying out the table
+ view, due to AppKit logic that attempts to adjust the size of the table view's enclosing scroll view after we've
+ already attempted to set the frame of the scroll view under the overridden call to -layout.
+
+ Fix this by refactoring the logic around the suggestion table view's enclosing scroll view; currently,
+ WKDataListSuggestionTableView overrides -enclosingScrollView to return its own NSScrollView, which it sizes
+ underneath an overridden call to -layout. This is a bit strange, since the table view is actually a subview of
+ the scroll view it owns; the fact that laying out the table view causes an ancestor view to change size (and
+ subsequently invalidate the table view's layout) seems to be what breaks AppKit's enclosing scroll view
+ adjustment logic.
+
+ Instead, we can have own both the table view and the table view's enclosing scroll view.
+
+ * UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:
+ (-[WKDataListSuggestionTableView initWithElementRect:]):
+ (-[WKDataListSuggestionsController initWithInformation:inView:]):
+
+ Move the scroll view initialization logic out of -[WKDataListSuggestionTableView initWithElementRect:] and into
+ this method. This also means that we can initialize the scroll view with the bounds of the window's content
+ view, instead of waiting until we lay out the table view underneath the scroll view.
+
+ (-[WKDataListSuggestionsController updateWithInformation:]):
+ (-[WKDataListSuggestionsController invalidate]):
+
+ Clear out and remove _scrollView as well when we tear down the controller.
+
+ (-[WKDataListSuggestionsController showSuggestionsDropdown:]):
+ (-[WKDataListSuggestionTableView layout]): Deleted.
+ (-[WKDataListSuggestionTableView enclosingScrollView]): Deleted.
+ (-[WKDataListSuggestionTableView removeFromSuperviewWithoutNeedingDisplay]): Deleted.
+
+ Now that the controller owns the scroll view, we don't need to override these anymore.
+
2020-03-29 Antoine Quint <[email protected]>
[AutoSizing] Bring back the old auto-sizing code as a deprecated codepath for compatibility reasons
Modified: trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm (259197 => 259198)
--- trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm 2020-03-30 13:49:44 UTC (rev 259197)
+++ trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm 2020-03-30 13:56:32 UTC (rev 259198)
@@ -221,9 +221,7 @@
@end
-@implementation WKDataListSuggestionTableView {
- RetainPtr<NSScrollView> _enclosingScrollView;
-}
+@implementation WKDataListSuggestionTableView
- (id)initWithElementRect:(const WebCore::IntRect&)rect
{
@@ -238,24 +236,9 @@
[column setWidth:rect.width()];
[self addTableColumn:column.get()];
- _enclosingScrollView = adoptNS([[NSScrollView alloc] init]);
- [_enclosingScrollView setHasVerticalScroller:YES];
- [_enclosingScrollView setVerticalScrollElasticity:NSScrollElasticityAllowed];
- [_enclosingScrollView setHorizontalScrollElasticity:NSScrollElasticityNone];
- [_enclosingScrollView setDocumentView:self];
- [_enclosingScrollView setDrawsBackground:NO];
- [[_enclosingScrollView contentView] setAutomaticallyAdjustsContentInsets:NO];
- [[_enclosingScrollView contentView] setContentInsets:NSEdgeInsetsMake(dropdownVerticalPadding, 0, dropdownVerticalPadding, 0)];
- [[_enclosingScrollView contentView] scrollToPoint:NSMakePoint(0, -dropdownVerticalPadding)];
-
return self;
}
-- (void)layout
-{
- [_enclosingScrollView setFrame:[_enclosingScrollView superview].bounds];
-}
-
- (void)reload
{
[self reloadData];
@@ -266,17 +249,6 @@
return NO;
}
-- (NSScrollView *)enclosingScrollView
-{
- return _enclosingScrollView.get();
-}
-
-- (void)removeFromSuperviewWithoutNeedingDisplay
-{
- [super removeFromSuperviewWithoutNeedingDisplay];
- [_enclosingScrollView removeFromSuperviewWithoutNeedingDisplay];
-}
-
@end
@implementation WKDataListSuggestionsController {
@@ -284,6 +256,7 @@
Vector<String> _suggestions;
NSView *_presentingView;
+ RetainPtr<NSScrollView> _scrollView;
RetainPtr<WKDataListSuggestionWindow> _enclosingWindow;
RetainPtr<WKDataListSuggestionTableView> _table;
}
@@ -306,6 +279,16 @@
[_enclosingWindow setBackgroundColor:[NSColor clearColor]];
[_enclosingWindow setOpaque:NO];
+ _scrollView = adoptNS([[NSScrollView alloc] initWithFrame:[_enclosingWindow contentView].bounds]);
+ [_scrollView setHasVerticalScroller:YES];
+ [_scrollView setVerticalScrollElasticity:NSScrollElasticityAllowed];
+ [_scrollView setHorizontalScrollElasticity:NSScrollElasticityNone];
+ [_scrollView setDocumentView:_table.get()];
+ [_scrollView setDrawsBackground:NO];
+ [[_scrollView contentView] setAutomaticallyAdjustsContentInsets:NO];
+ [[_scrollView contentView] setContentInsets:NSEdgeInsetsMake(dropdownVerticalPadding, 0, dropdownVerticalPadding, 0)];
+ [[_scrollView contentView] scrollToPoint:NSMakePoint(0, -dropdownVerticalPadding)];
+
[_table setDelegate:self];
[_table setDataSource:self];
[_table setAction:@selector(selectedRow:)];
@@ -330,6 +313,7 @@
[_table reload];
[_enclosingWindow setFrame:[self dropdownRectForElementRect:information.elementRect] display:YES];
+ [_scrollView setFrame:[_enclosingWindow contentView].bounds];
}
- (void)notifyAccessibilityClients:(NSString *)info
@@ -364,6 +348,7 @@
- (void)invalidate
{
[_table removeFromSuperviewWithoutNeedingDisplay];
+ [_scrollView removeFromSuperviewWithoutNeedingDisplay];
[_table setDelegate:nil];
[_table setDataSource:nil];
@@ -370,6 +355,7 @@
[_table setTarget:nil];
_table = nil;
+ _scrollView = nil;
[[_presentingView window] removeChildWindow:_enclosingWindow.get()];
[_enclosingWindow close];
@@ -391,10 +377,10 @@
- (void)showSuggestionsDropdown:(WebKit::WebDataListSuggestionsDropdownMac*)dropdown
{
_dropdown = dropdown;
- [[_enclosingWindow contentView] addSubview:[_table enclosingScrollView]];
+ [[_enclosingWindow contentView] addSubview:_scrollView.get()];
[_table reload];
[[_presentingView window] addChildWindow:_enclosingWindow.get() ordered:NSWindowAbove];
- [[_table enclosingScrollView] flashScrollers];
+ [_scrollView flashScrollers];
// Notify accessibility clients of datalist becoming visible.
NSString *currentSelectedString = [self currentSelectedString];
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
