Bug 162471

Summary: AX: iOS: Tapping <input> in Safari zooms in a bit when page has max scale = 1
Product: WebKit Reporter: Nan Wang <n_wang>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, commit-queue, n_wang, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch
none
patch
none
patch
none
patch
none
patch none

Description Nan Wang 2016-09-22 16:17:36 PDT
We were ignoring force always user scalable for input field but we missed a case where web authors sometimes set maximum scale to 1 instead of user-scalable=no.

<rdar://problem/27137434>
Comment 1 Nan Wang 2016-09-22 16:29:33 PDT
Created attachment 289628 [details]
patch
Comment 2 WebKit Commit Bot 2016-09-22 16:30:26 PDT
Attachment 289628 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Nan Wang 2016-09-22 16:30:45 PDT
Created attachment 289629 [details]
patch

fixed a typo
Comment 4 WebKit Commit Bot 2016-09-22 16:33:11 PDT
Attachment 289629 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Nan Wang 2016-09-22 17:21:08 PDT
Created attachment 289634 [details]
patch

fixing timeout tests.
Comment 6 WebKit Commit Bot 2016-09-22 17:22:02 PDT
Attachment 289634 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Nan Wang 2016-09-22 18:15:13 PDT
Created attachment 289640 [details]
patch

let's fix the tests again :(
Comment 8 WebKit Commit Bot 2016-09-22 18:17:39 PDT
Attachment 289640 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Nan Wang 2016-09-22 19:08:30 PDT
Created attachment 289648 [details]
patch

I figured touch events are not supported in layout tests
Comment 10 WebKit Commit Bot 2016-09-22 19:10:02 PDT
Attachment 289648 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Nan Wang 2016-09-22 22:03:49 PDT
Created attachment 289662 [details]
patch

Moved the tests to the right place.
Comment 12 WebKit Commit Bot 2016-09-22 22:05:42 PDT
Attachment 289662 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Nan Wang 2016-09-22 23:40:16 PDT
Created attachment 289665 [details]
patch

Actually those tests were failing on my local machine after moving to a different place :(
Now fixed it, should be good...
Comment 14 WebKit Commit Bot 2016-09-22 23:42:32 PDT
Attachment 289665 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Simon Fraser (smfr) 2016-09-29 14:38:26 PDT
Comment on attachment 289665 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289665&action=review

> Source/WebCore/page/ViewportConfiguration.h:95
> +    double maximumScaleIgnoringForceAlwaysScaling() const { return m_configuration.maximumScale; }

Maybe "maximumScaleIgnoringAlwaysScalable()"

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:332
> +    if (!m_viewportConfiguration.allowsUserScalingIgnoringForceAlwaysScaling())

I literally cannot parse "allowsUserScalingIgnoringForceAlwaysScaling"

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:333
> +        return m_page->pageScaleFactor();

This seems wrong. Shouldn't this return m_viewportConfiguration.maximumScale() ?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2553
> +    information.maximumScaleFactorIgnoringForceAlwaysScaling = maximumPageScaleFactorIgnoringForceAlwaysScaling();

Can we use "ignoringForceAlwaysScalable everywhere?
Comment 16 Nan Wang 2016-09-29 14:46:54 PDT
Comment on attachment 289665 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289665&action=review

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:332
>> +    if (!m_viewportConfiguration.allowsUserScalingIgnoringForceAlwaysScaling())
> 
> I literally cannot parse "allowsUserScalingIgnoringForceAlwaysScaling"

I'll try to rename this with ignoringForceAlwaysScalable

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:333
>> +        return m_page->pageScaleFactor();
> 
> This seems wrong. Shouldn't this return m_viewportConfiguration.maximumScale() ?

I'm not an expert on this but I copied the logic from WebPage::maximumPageScaleFactor()

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2553
>> +    information.maximumScaleFactorIgnoringForceAlwaysScaling = maximumPageScaleFactorIgnoringForceAlwaysScaling();
> 
> Can we use "ignoringForceAlwaysScalable everywhere?

ok
Comment 17 Nan Wang 2016-09-29 15:51:51 PDT
Created attachment 290257 [details]
patch

Renamed IgnoringForceAlwaysScaling to IgnoringAlwaysScalable.
Comment 18 WebKit Commit Bot 2016-09-29 15:54:07 PDT
Attachment 290257 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1105:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1106:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 WebKit Commit Bot 2016-09-29 17:40:42 PDT
Comment on attachment 290257 [details]
patch

Clearing flags on attachment: 290257

Committed r206626: <http://trac.webkit.org/changeset/206626>
Comment 20 WebKit Commit Bot 2016-09-29 17:40:47 PDT
All reviewed patches have been landed.  Closing bug.