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 NTP background images are the wrong aspect ratio #28450

Closed
petemill opened this issue Feb 11, 2023 · 3 comments · Fixed by brave/brave-core#17186
Closed

Android NTP background images are the wrong aspect ratio #28450

petemill opened this issue Feb 11, 2023 · 3 comments · Fixed by brave/brave-core#17186

Comments

@petemill
Copy link
Member

petemill commented Feb 11, 2023

There have been a few reports that NTP images, both sponsored and regular, are being stretched. In portrait mode, that appears to usually make the image wider than it should be.

What we have found out:

  • The amount of stretch seems to be dependent on the device size.
  • The issue does not happen if the Brave News feature flag is disabled.

The issue seems to be caused by creating an image that is the device width and height, but setting it directly on the FrameLayout. The bug was possibly caused in brave/brave-core#13951 because previous to that, the image was being set on a scroll view.

However, it seems that possibly because of the refactor in brave/brave-core#13951 we don't even need to be using the code path involving Glide and the FrameLayout at all:

Bypassing the code that calls BraveActivity.getBraveActivity().setBackground(bgWallpaper); fixes the issue and doesn't seem to cause any bugs scrolling down to Brave News. So perhaps that code path was only needed when the scroll view was used.

        public void bgWallpaperRetrieved(Bitmap bgWallpaper) {
            // if (ChromeFeatureList.isEnabled(BraveFeatureList.BRAVE_NEWS)) {
                // if (BraveActivity.getBraveActivity() != null && mTabProvider.get() != null
                        // && !mTabProvider.get().isIncognito()) {
                    // BraveActivity.getBraveActivity().setBackground(bgWallpaper);
                // }

            // } else {
                mBgImageView.setImageBitmap(bgWallpaper);
            // }
        }

Test Plan

TBD - something that measures the "squareness" or the aspect ratio of something identifiable on one of the bg images.

Maybe we can coordinate a test using --use-dev-goupdater-url in Slack in the #ntp-sponsored-image channel? (employee only)

@kjozwiak
Copy link
Member

The above requires 1.48.170 or higher for 1.48.x verification 👍 @deeppandya will also help verify this on his Pixel 7 Pro device that originally reproduced the above issue as well.

@Uni-verse
Copy link
Collaborator

Uni-verse commented Feb 23, 2023

Verification Passed on Pixel 7 Pro using the version 1.48.171 per https://bravesoftware.slack.com/archives/C0YL5KMA8/p1677164300099009?thread_ts=1677135710.642299&cid=C0YL5KMA8

screen-20230223-202548.mp4

cc: @deeppandya

@Uni-verse
Copy link
Collaborator

Verified on Samsung GS 21 5G using version:

Brave	1.48.171 Chromium: 110.0.5481.177 (Official Build) (64-bit) 
Revision	f34f7ab2d4ca4ad498ef42aeba4f4eb2c1392d63-refs/branch-heads/5481@{#1239}
OS	Android 13; Build/TP1A.220624.014
  • Ensured that NTP SI are displayed properly and have the correct aspect ratio
Example Example Example Example
screenshot-1677189188650 screenshot-1677189197513 screenshot-1677189210272 screenshot-1677189224288

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.

6 participants