`View.getGlobalVisibleRect()` is broken in some use cases

[d4vidi]

🐛 Bug Report

With respect to the list of these commits - all of which involve child-views clipping on Android (AKA the overflow functionality): b81c8b, 6110a4c, bbdc12e, 9c71952;

While in essence the approach makes sense for introducing overflow toggling support, having view-group components' clip-children flag hard-codedly set to false creates undesired side-effects. One of which is the breaking of the logic of the native View.getGlobalVisibleRect() method. In some cases -- as illustrated by the screenshot from a demo app I've worked out for this, the native method will return true for views that were effectively properly clipped by ReactViewGroup, and are in fact not visible on the screen.

In that specific use case, while the view-group holding the upper part of the screen (gray background) is limited in height, the scroll view stretches all the way down to the bottom of the screen. This way, items 14..17 are drawn under the lower part of the screen (blue), and - as far as Android is concerned, are NOT clipped by the parent view-group -- as it's clip-children flag is off.

Note: the bug persists even if ScrollView is removed from the hierarchy.

Among other potential functionalities, this stability of the View.getGlobalVisibleRect() method is paramount for ui-testing projects to work -- such as https://github.com/wix/Detox (the one I'm working on right now) and that is already integrated into React Native itself (for iOS).

To Reproduce

The demo app is available on github

Expected Behavior

View.getGlobalVisibleRect() should return false for views that were clipped off by one of their view-group parents.

Code Example

From the demo app:

• A screen layout where this bug occurs
• The native logic that runs 'under the hood' in order to show the bug is there.

Environment

info 
  React Native Environment Info:
    System:
      OS: macOS 10.14.3
      CPU: (8) x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
      Memory: 409.30 MB / 16.00 GB
      Shell: 3.2.57 - /bin/bash
    Binaries:
      Node: 9.11.2 - ~/.nvm/versions/node/v9.11.2/bin/node
      Yarn: 1.3.2 - /usr/local/bin/yarn
      npm: 6.8.0 - ~/.nvm/versions/node/v9.11.2/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
      Android SDK:
        API Levels: 19, 21, 22, 23, 24, 25, 26, 27, 28
        Build Tools: 19.1.0, 20.0.0, 21.1.2, 22.0.1, 23.0.1, 23.0.2, 23.0.3, 24.0.1, 25.0.0, 25.0.1, 25.0.2, 25.0.3, 26.0.0, 26.0.1, 26.0.2, 26.0.3, 27.0.1, 27.0.3, 28.0.0, 28.0.2, 28.0.3
        System Images: android-16 | Google APIs Intel x86 Atom, android-19 | Google APIs Intel x86 Atom, android-22 | Google APIs Intel x86 Atom, android-22 | Google APIs Intel x86 Atom_64, android-23 | Google APIs Intel x86 Atom_64, android-25 | Google APIs Intel x86 Atom_64, android-26 | Google APIs Intel x86 Atom, android-26 | Google Play Intel x86 Atom, android-28 | Google Play Intel x86 Atom
    IDEs:
      Android Studio: 3.3 AI-182.5107.16.33.5199772
      Xcode: 10.1/10B61 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.3 => 16.8.3 
      react-native: 0.59.0 => 0.59.0 
    npmGlobalPackages:
      react-native-cli: 2.0.1

[react-native-bot]

It looks like you are using an older version of React Native. Please update to the latest release, v0.59 and verify if the issue still exists.

The "Resolution: Old Version" label will be removed automatically once you edit your original post with the results of running react-native info on a project using the latest release.

[d4vidi]

PLEASE IGNORE THE BOT, I've long had the react native info upgraded to 0.59.
Please give me some lovin' with this one.

@dulmandakh @hramos

[heroic]

+1

@axe-fb @cpojer

[yungsters]

There is a blog post titled, Android View Measurement. It provides a very good explanation of getGlobalVisibleRect() with its caveats:

getGlobalVisibleRect() is quite misleading, it actually pertains to clipping, not visibility.

If we take a look at the implementation of getGlobalVisibleRect(), we see:

• getGlobalVisibleRect() delegates to mParent.getChildVisibleRect().
• mParent.getChildVisibleRect() simply intersects its own dimensions with the child's.

I do not have particular opinions about this Android API. But I can understand why, from a performance perspective, the platform designers would not want to provide a simple method call that does an O(depth) traversal.

If you want a method that recursively traverses parent view groups to apply all clippings, it would appear that you can do this by repeatedly calling getChildVisibleRect() on getParent(). Would that work?

[d4vidi]

@yungsters I don't think that would be helpful tbh, as, given the existing implementation, getGlobalVisibleRect() doesn't work even if the direct parent itself is the one doing the clipping (O(1) check, no tree traversal).

I suppose the real question here is whether hard-setting native views' clipping to false while implementing custom clipping in a custom draw implementation is a fair strategy... In essence, it puts the Android-ish UI realm under the wrong notion of that (almost) nothing clips anything else, such that no matter what API I use, it would't be accurate.
As I understand it, the only way to introduce a query of that sort that would actually work is through react native's implementation itself, which has become the one real source of truth in this case...
wdyt?