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

Brave News Customize dialog is blank #27052

Closed
stephendonner opened this issue Nov 29, 2022 · 6 comments · Fixed by brave/brave-core#16159
Closed

Brave News Customize dialog is blank #27052

stephendonner opened this issue Nov 29, 2022 · 6 comments · Fixed by brave/brave-core#16159

Comments

@stephendonner
Copy link

stephendonner commented Nov 29, 2022

Description

Brave News Customize dialog is blank

Found while testing brave/brave-core#16086.

Steps to Reproduce

  1. install 1.48.15
  2. launch Brave
  3. load theatlantic.com
  4. click on the brand-new RSS-feed icon in the URL bar
  5. click on Best of The Atlantic
  6. click on Manage feeds…
  7. click Turn on Brave News

Actual result:

Blank Customize Brave News dialog

Screencast:

blank-page

Expected result:

Customize Brave News dialog, populated with theatlantic.com Best of the Atlantic feed shown under Following

Reproduces how often:

100%

Brave version (brave://version info)

Brave 1.48.15 Chromium: 108.0.5359.62 (Official Build) nightly (x86_64)
Revision 041930a89a990cfab0315a2d9f20d6429a4a67cf-refs/branch-heads/5359@{#938}
OS macOS Version 11.7.1 (Build 20G918)

Version/Channel Information:

  • Can you reproduce this issue with the current release? No
  • Can you reproduce this issue with the beta channel? Unknown
  • Can you reproduce this issue with the nightly channel? Yes

/cc @fallaciousreasoning @petemill @mattmcalister @rebron @brave/qa-team

@fallaciousreasoning
Copy link

fallaciousreasoning commented Nov 30, 2022

Okay, I have a simpler repro which doesn't require a new profile:

  1. Subscribe to a feed
  2. Ensured Brave News is disabled (via the toggle on the ntp)
  3. Refresh
  4. Enable Brave News

This was being caused because you can get suggestions before you get publishers (so we'd have publisherId for the suggestion but no info about the publisher).

To fix this, I'm pushing the data into the NTP every time the publisher info is updated, which we probably should've been doing anyway.

I think this was introduced by brave/brave-core#16038, so the fix needs to be uplifted to 1.47.x

@stephendonner
Copy link
Author

Sadly, this still happens, occasionally, using:

Brave 1.48.132 Chromium: 109.0.5414.87 (Official Build) beta (x86_64)
Revision 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS macOS Version 11.7.2 (Build 20G1020)

It's really tough to reproduce at will, so maybe a race condition?

blank tab new tab
Screen Shot 2023-01-19 at 12 59 38 PM Screen Shot 2023-01-19 at 1 00 45 PM

@petemill
Copy link
Member

@fallaciousreasoning I wonder if that could be caused by this line https://github.com/brave/brave-core/blob/fc5f30881335f21f1aee93c9b14d84f789ecd013/components/brave_new_tab_ui/components/default/braveToday/customize/FeedCard.tsx#L72

I haven't traced calls to that function to see if it's likely Publisher could be null, but I couldn't find many other relevant calls from that error in @stephendonner 's screenshot.

@fallaciousreasoning
Copy link

I think that's probably accurate @petemill - just curious how we're getting into that state. @stephendonner came up with a good repro so I might have a bit more of a dig (and guard that line too, once I've fixed things).

From @stephendonner

  1. new profile
  2. launch Brave
  3. open a new-tab page
  4. scroll down on the Brave News opt-in dialog, click Learn More (this replaces the tab’s content)
  5. now click the Back button on the browser
  6. click Show Brave News
  7. click Customize

@kjozwiak
Copy link
Member

kjozwiak commented Jan 19, 2023

Closing as we don't re-open issues that already have an landed PRs. So we'll need a new follow up issue which @stephendonner will create with the new STR/Cases he mentioned above. If this one isn't working, we can label this as QA/No and release-notes/exclude and QA can run through the above verification on the new issue that will get created and most likely uplifted.

The reason we don't re-open issues that it eventually gets too confusing once you start re-opening/closing an issue that has multiple PRs and uplifts associated with it. For example, in this case, we now have an issue that was supposed to fix an issue within 1.48.x and was moved into that milestone but now also describes another issue within the master.

@stephendonner let us know once you've created the new issue 👍 Can mention it's a follow up to this one.

@stephendonner
Copy link
Author

@stephendonner let us know once you've created the new issue 👍 Can mention it's a follow up to this one.

Done; logged #27914 👍

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