Skip to content

Commit

Permalink
[Uplift] [1.49]: Brave News sometimes picks the incorrect language (#…
Browse files Browse the repository at this point in the history
…17633)

Brave News sometimes picks the wrong language in the Subscribe Button (#17046)

* Fix bug where first source was returned instead of the one in default locale

* Update JSON

* Add tests

* Remove string locale support (it isn't used)

* Changes for PR
  • Loading branch information
fallaciousreasoning authored Mar 20, 2023
1 parent 58125b9 commit 2e43001
Show file tree
Hide file tree
Showing 3 changed files with 242 additions and 53 deletions.
80 changes: 56 additions & 24 deletions components/brave_today/browser/publishers_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "brave/components/brave_today/browser/publishers_controller.h"

#include <algorithm>
#include <cstddef>
#include <iterator>
#include <memory>
Expand All @@ -15,8 +16,10 @@
#include "base/barrier_callback.h"
#include "base/containers/contains.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/location.h"
#include "base/one_shot_event.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "brave/components/api_request_helper/api_request_helper.h"
#include "brave/components/brave_private_cdn/headers.h"
Expand All @@ -34,6 +37,41 @@

namespace brave_news {

namespace {
mojom::Publisher* FindMatchPreferringLocale(
const Publishers& publishers,
const std::string& preferred_locale,
base::RepeatingCallback<bool(const mojom::Publisher&)> matcher) {
if (publishers.empty()) {
return nullptr;
}

mojom::Publisher* match = nullptr;
for (const auto& [_, publisher] : publishers) {
if (!matcher.Run(*publisher)) {
continue;
}

auto locale_it = base::ranges::find(publisher->locales, preferred_locale,
&mojom::LocaleInfo::locale);
// If the match is in our preferred locale, return it.
if (locale_it != publisher->locales.end()) {
return publisher.get();
}

// Otherwise, if we don't have a match yet, make this our match (so we
// prefer whatever we found first).
if (!match) {
match = publisher.get();
}
}

// If we didn't have a match in our preferred locale, return the first
// matching publisher.
return match;
}
} // namespace

PublishersController::PublishersController(
PrefService* prefs,
DirectFeedController* direct_feed_controller,
Expand All @@ -49,39 +87,31 @@ PublishersController::~PublishersController() = default;

const mojom::Publisher* PublishersController::GetPublisherForSite(
const GURL& site_url) const {
if (publishers_.empty())
return nullptr;

const auto& site_host = site_url.host();

// Can't match a Publisher from an empty host
if (site_host.empty()) {
return nullptr;
}

for (const auto& kv : publishers_) {
const auto& publisher_host = kv.second->site_url.host();
// When https://github.com/brave/brave-browser/issues/26092 is fixed, this
// hack can be removed.
const auto& publisher_host_www = "www." + publisher_host;
if (publisher_host == site_host || publisher_host_www == site_host) {
return kv.second.get();
}
}

return nullptr;
return FindMatchPreferringLocale(
publishers_, default_locale_,
base::BindRepeating(
[](const std::string& site_host, const mojom::Publisher& publisher) {
return publisher.site_url.host() == site_host;
},
site_host));
}

const mojom::Publisher* PublishersController::GetPublisherForFeed(
const GURL& feed_url) const {
if (publishers_.empty())
return nullptr;

for (const auto& kv : publishers_) {
if (kv.second->feed_source == feed_url)
return kv.second.get();
}
return nullptr;
return FindMatchPreferringLocale(
publishers_, default_locale_,
base::BindRepeating(
[](const GURL& feed_url, const mojom::Publisher& publisher) {
return publisher.feed_source == feed_url;
},
feed_url));
}

const Publishers& PublishersController::GetLastPublishers() const {
Expand Down Expand Up @@ -185,8 +215,9 @@ void PublishersController::EnsurePublishersIsUpdating() {
"Attempting to migrate to a direct feed.";
// We only care about missing publishers if the user was subscribed
// to them.
if (is_user_enabled.value_or(false))
if (is_user_enabled.value_or(false)) {
missing_publishers_.push_back(publisher_id);
}
}
}
// Add direct feeds
Expand Down Expand Up @@ -225,8 +256,9 @@ void PublishersController::EnsurePublishersIsUpdating() {
uint64_t migrated_count) {
// If any publisher was migrated, ensure we update the list
// of publishers.
if (migrated_count != 0)
if (migrated_count != 0) {
controller->EnsurePublishersIsUpdating();
}
},
base::Unretained(controller)));
}
Expand Down
Loading

0 comments on commit 2e43001

Please sign in to comment.