Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Content is missing on long posts in html mode #1268

Closed
marecar3 opened this issue Aug 7, 2019 · 4 comments
Closed

[Android] Content is missing on long posts in html mode #1268

marecar3 opened this issue Aug 7, 2019 · 4 comments
Assignees

Comments

@marecar3
Copy link
Contributor

marecar3 commented Aug 7, 2019

If there is some long post that should be presented in HTML mode e.g. https://gist.github.com/hypest/25e6d96b9c45ce92770f3c4c7ccafa9d

HTML editor won't show any content as a result of: ReactEditText not displayed because it is too large to fit into a software layer (or drawing cache), needs 18616320 bytes, only 18432000 available

@marecar3
Copy link
Contributor Author

marecar3 commented Aug 16, 2019

There is an active PR: #1277
which solves the issue.

But as it's explained in this comment: #1277 (comment)
we would need to go deeper and then to see if we can solve it in a better way.

@marecar3 marecar3 removed their assignment Sep 24, 2019
@daniloercoli daniloercoli self-assigned this Sep 25, 2019
@daniloercoli
Copy link
Contributor

I've investigated the issue as part of the current sprint and think i found something interesting:

  • The issue is only available on Android 8.X
    I was not able to reproduce it on devices running Android 7, and Android 9.

  • As already mentioned in the original proposed PR, even if we set the whole ReactView to "software layer", the warning W/View: ReactEditText not displayed because it is too large to fit into a software layer (or drawing cache), needs 112216320 bytes, only 8294400 is still available in the log.

  • The ReactEditText component already has a workaround that sets its layer type to "software". See here.

With those above in mind I started to break our app by changing the native/web code, until I found that the culprit is the ScrollView that wraps the TextView used to display html code, added to give to TextView scrolling capabilities. See here.

  • If you replace ScrollView with the simpler View component, the HTML code is then correctly displayed, although no scrolling abilities. Very hard to scroll down.
  • I tried with no luck to keep ScrollView in place, and change the CSS styles applied to it.

My conclusions:
I think the underlying problem is that ScrollView on Android 8 doesn't have the forced software layer "patch" applied to it, like TextView has. I checked the ReactNative code, and in fact, that's the case.

To fix the problem we have 2 ways:

  • On the host app - wp-android - We can apply to mReactRootView the layer type to "software" only under some circumstances. EX: Android 8 detected, and user switched to HTML mode. Restore it back to Hardware when it's done with the HTML mode.
    public void toggleEditorMode(boolean htmlModeEnabled) {
        // Turn off hardware acceleration for Oreo (T40484798)
        // see https://issuetracker.google.com/issues/67102093
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O
            && Build.VERSION.SDK_INT <= Build.VERSION_CODES.O_MR1) {
            if (htmlModeEnabled) {
                mReactRootView.setLayerType(View.LAYER_TYPE_SOFTWARE, null);
            } else {
                mReactRootView.setLayerType(View.LAYER_TYPE_HARDWARE, null);
            }
        }
       mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().toggleEditorMode();
    }
  • Extend the ReactNative ScrollView component, and sets its layer type to "software" on Android 8. In order to do this, I think we need to write our own ScrollView component that extends the react base ScrollView....and basically create another mini project.
    This in my opinion is the most clean solution, even though adds much complexity.
    Maybe we can try to propose a patch to RN directly, but I guess the memory problems are hard to reproduce in demo apps.

For reference, this is the testing post i'm using on my blog. I deliberately chose to use a simple long post with para blocks only.

@daniloercoli
Copy link
Contributor

I've spent some time writing our own version of WPScrollView that does extend the RN ScrollView component, and sets HW acceleration OFF on Android 8.
See: try/WPScrollManager-Android and the GB side of it.

Unfortunately, it seems to me, that the way the original ScrollView component is structured, doesn't give us the ability to extend it, and override the native class used to implement the scrolling see: https://github.com/facebook/react-native/blob/master/Libraries/Components/ScrollView/ScrollView.js#L996
I tried in our component to override the value of AndroidHorizontalScrollView, with no luck. Maybe there is a way to do that cc @Tug - but the value of AndroidHorizontalScrollView is defined outside the component and it does't give the ability to set a custom class by calling one of its methods.
Without this, our WPScrollView - native side - is not invoked. Our component is used JS side, and properly called, but internally it uses the code from its superclass, that does instantiate the RN native default ScrollView.

cc @hypest Now sure, but at this point i'm prone toward the first solution outlined in my previous comment, that does disable HW on Android 8 only, when HTML mode is switched, and left any further investigation to future time. I guess we need to fix this issue anyway.

@daniloercoli
Copy link
Contributor

Fixed in #1401 - Maybe we need to reiterate on this and propose a better solution, but for now we're ok with the current one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants