diff --git a/browser/brave_news/BUILD.gn b/browser/brave_news/BUILD.gn index 036c50885a4e..636946e3099b 100644 --- a/browser/brave_news/BUILD.gn +++ b/browser/brave_news/BUILD.gn @@ -9,11 +9,12 @@ source_set("brave_news") { "//brave/browser/profiles:util", "//brave/components/brave_today/browser", "//brave/components/brave_today/common", - "//brave/components/brave_today/common:common", "//brave/components/brave_today/common:mojom", "//chrome/browser:browser", + "//chrome/browser/favicon", + "//chrome/browser/profiles", "//chrome/browser/profiles:profile", - "//chrome/browser/profiles:profiles", + "//components/favicon/core", "//components/keyed_service/content", "//components/user_prefs", "//content/public/browser", diff --git a/browser/brave_news/brave_news_controller_factory.cc b/browser/brave_news/brave_news_controller_factory.cc index 81f3b6bb7100..6240eade47aa 100644 --- a/browser/brave_news/brave_news_controller_factory.cc +++ b/browser/brave_news/brave_news_controller_factory.cc @@ -8,10 +8,13 @@ #include "brave/browser/brave_ads/ads_service_factory.h" #include "brave/browser/profiles/profile_util.h" #include "brave/components/brave_today/browser/brave_news_controller.h" +#include "chrome/browser/favicon/favicon_service_factory.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/profile.h" +#include "components/favicon/core/favicon_service.h" #include "components/keyed_service/content/browser_context_dependency_manager.h" +#include "components/keyed_service/core/service_access_type.h" #include "content/public/browser/browser_context.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -49,6 +52,7 @@ BraveNewsControllerFactory::BraveNewsControllerFactory() "BraveNewsControllerFactory", BrowserContextDependencyManager::GetInstance()) { DependsOn(brave_ads::AdsServiceFactory::GetInstance()); + DependsOn(FaviconServiceFactory::GetInstance()); DependsOn(HistoryServiceFactory::GetInstance()); } @@ -63,11 +67,13 @@ KeyedService* BraveNewsControllerFactory::BuildServiceInstanceFor( if (!profile) { return nullptr; } + auto* favicon_service = FaviconServiceFactory::GetForProfile( + profile, ServiceAccessType::EXPLICIT_ACCESS); auto* ads_service = brave_ads::AdsServiceFactory::GetForProfile(profile); auto* history_service = HistoryServiceFactory::GetForProfile( profile, ServiceAccessType::EXPLICIT_ACCESS); - return new BraveNewsController(profile->GetPrefs(), ads_service, - history_service, + return new BraveNewsController(profile->GetPrefs(), favicon_service, + ads_service, history_service, profile->GetURLLoaderFactory()); } diff --git a/components/brave_new_tab_ui/api/brave_news/news.ts b/components/brave_new_tab_ui/api/brave_news/news.ts index a2508415de64..c8c17e3344c4 100644 --- a/components/brave_new_tab_ui/api/brave_news/news.ts +++ b/components/brave_new_tab_ui/api/brave_news/news.ts @@ -171,4 +171,5 @@ class BraveNewsApi { } } -export const api = new BraveNewsApi() +export const api = new BraveNewsApi(); +(window as any).api = api diff --git a/components/brave_new_tab_ui/components/default/braveToday/customize/Configure.tsx b/components/brave_new_tab_ui/components/default/braveToday/customize/Configure.tsx index 3742ae6c3bdc..5fbdc606c133 100644 --- a/components/brave_new_tab_ui/components/default/braveToday/customize/Configure.tsx +++ b/components/brave_new_tab_ui/components/default/braveToday/customize/Configure.tsx @@ -23,6 +23,9 @@ const Grid = styled.div` width: 100%; height: 100%; + overflow: auto; + overscroll-behavior: none; + display: grid; grid-template-columns: 250px auto; grid-template-rows: 64px 2px auto; diff --git a/components/brave_new_tab_ui/components/default/braveToday/customize/SourcesListEntry.tsx b/components/brave_new_tab_ui/components/default/braveToday/customize/SourcesListEntry.tsx index 5668c0f187eb..de331815b933 100644 --- a/components/brave_new_tab_ui/components/default/braveToday/customize/SourcesListEntry.tsx +++ b/components/brave_new_tab_ui/components/default/braveToday/customize/SourcesListEntry.tsx @@ -8,7 +8,7 @@ import styled from 'styled-components' import { getLocale } from '$web-common/locale' import Flex from '../../../Flex' import { useChannelSubscribed, usePublisher, usePublisherFollowed } from './Context' -import { useLazyUnpaddedImageUrl } from '../useUnpaddedImageUrl' +import { useLazyFavicon } from '../useUnpaddedImageUrl' import { getTranslatedChannelName } from './ChannelCard' interface Props { @@ -61,11 +61,10 @@ const ChannelNameText = styled.span` font-weight: 600; ` -function FavIcon (props: { src?: string }) { - const { url, setElementRef } = useLazyUnpaddedImageUrl(props.src, { +function FavIcon (props: { publisherId: string }) { + const { url, setElementRef } = useLazyFavicon(props.publisherId, { rootElement: document.getElementById('brave-news-configure'), - rootMargin: '0px 0px 100px 0px', - useCache: true + rootMargin: '200px 0px 200px 0px' }) const [error, setError] = React.useState(false) return @@ -79,7 +78,7 @@ export function FeedListEntry (props: Props) { return - + {publisher.publisherName} setFollowed(false)}> diff --git a/components/brave_new_tab_ui/components/default/braveToday/customize/useSearch.ts b/components/brave_new_tab_ui/components/default/braveToday/customize/useSearch.ts index 6bcad5528892..bd46488cae0e 100644 --- a/components/brave_new_tab_ui/components/default/braveToday/customize/useSearch.ts +++ b/components/brave_new_tab_ui/components/default/braveToday/customize/useSearch.ts @@ -39,11 +39,16 @@ export default function useSearch (query: string) { p.feedSource?.url?.toLocaleLowerCase().includes(lowerQuery))) const results = { publishers, direct: [] as FeedSearchResultItem[] } for (const result of directResults) { - const publisherMatch = publishers.find(p => p.feedSource.url === result.feedUrl.url) - if (publisherMatch && !publishers.some(p => p.publisherId === publisherMatch.publisherId)) { - results.publishers.push(publisherMatch) - } else { + // If we have any publisher with this feed url, we should prefer showing + // that to creating a new direct feed. + // Note: We should only add the publisher to the list of publisher matches + // if it isn't already matching. The canonical case here is + // news.com (redirects to cnet.com), so we should show the cnet.com feed. + const publisherMatch = context.sortedPublishers.find(p => p.feedSource.url === result.feedUrl.url) + if (!publisherMatch) { results.direct.push(result) + } else if (!results.publishers.some(p => p.publisherId === publisherMatch.publisherId)) { + results.publishers.push(publisherMatch) } } return { diff --git a/components/brave_new_tab_ui/components/default/braveToday/useUnpaddedImageUrl.ts b/components/brave_new_tab_ui/components/default/braveToday/useUnpaddedImageUrl.ts index e29159654ffc..3bcd943caeae 100644 --- a/components/brave_new_tab_ui/components/default/braveToday/useUnpaddedImageUrl.ts +++ b/components/brave_new_tab_ui/components/default/braveToday/useUnpaddedImageUrl.ts @@ -5,6 +5,7 @@ import * as React from 'react' import getBraveNewsController from '../../../api/brave_news' +import { useVisible, VisibleOptions } from '$web-common/useVisible' interface Options { rootElement?: HTMLElement | null @@ -16,6 +17,46 @@ interface Options { } const cache: { [url: string]: string } = {} +function useFavicon (publisherId: string | undefined) { + const [url, setUrl] = React.useState(cache[publisherId!]) + React.useEffect(() => { + if (!publisherId) { + return + } + + if (cache[publisherId]) { + setUrl(cache[publisherId]) + return + } + + let cancelled = false + getBraveNewsController().getFavIconData(publisherId).then(({ imageData }) => { + if (cancelled) return + if (!imageData) return + + const blob = new Blob([new Uint8Array(imageData)], { type: 'image/*' }) + const url = URL.createObjectURL(blob) + cache[publisherId] = url + setUrl(url) + }) + + return () => { + cancelled = true + } + }, [publisherId]) + + return url +} + +export function useLazyFavicon (publisherId: string, options: VisibleOptions) { + const { visible, setElementRef } = useVisible(options) + const url = useFavicon(visible ? publisherId : undefined) + return { + url, + setElementRef + } +} + export function useUnpaddedImageUrl (paddedUrl: string | undefined, onLoaded?: () => any, useCache?: boolean) { const [unpaddedUrl, setUnpaddedUrl] = React.useState('') @@ -67,28 +108,7 @@ export function useUnpaddedImageUrl (paddedUrl: string | undefined, onLoaded?: ( } export function useLazyUnpaddedImageUrl (paddedUrl: string | undefined, options: Options) { - const [visible, setVisible] = React.useState(false) - const [elementRef, setElementRef] = React.useState(null) - - React.useEffect(() => { - if (!elementRef) return - - const observer = new IntersectionObserver(([intersectionInfo]) => { - if (!intersectionInfo.isIntersecting) { - return - } - setVisible(true) - }, { - root: options.rootElement, - rootMargin: options.rootMargin, - threshold: options.threshold - }) - - observer.observe(elementRef) - return () => { - observer.disconnect() - } - }, [options.rootElement, elementRef]) + const { visible, setElementRef } = useVisible(options) return { url: useUnpaddedImageUrl(visible ? paddedUrl : undefined, options.onLoaded, options.useCache) || cache[paddedUrl!], diff --git a/components/brave_today/browser/BUILD.gn b/components/brave_today/browser/BUILD.gn index 7bea2bbd7fd4..c68d19533cde 100644 --- a/components/brave_today/browser/BUILD.gn +++ b/components/brave_today/browser/BUILD.gn @@ -45,6 +45,8 @@ static_library("browser") { "//brave/components/l10n/common", "//brave/components/p3a_utils", "//brave/components/time_period_storage", + "//components/favicon/core:core", + "//components/favicon_base:favicon_base", "//components/feed:buildflags", "//components/history/core/browser", "//components/keyed_service/core", diff --git a/components/brave_today/browser/brave_news_controller.cc b/components/brave_today/browser/brave_news_controller.cc index 4f46d2e8a35a..fdebeea46b38 100644 --- a/components/brave_today/browser/brave_news_controller.cc +++ b/components/brave_today/browser/brave_news_controller.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_today/browser/brave_news_controller.h" +#include #include #include #include @@ -37,6 +38,8 @@ #include "brave/components/brave_today/common/features.h" #include "brave/components/brave_today/common/pref_names.h" #include "brave/components/l10n/common/locale_util.h" +#include "components/favicon/core/favicon_service.h" +#include "components/favicon_base/favicon_types.h" #include "components/history/core/browser/history_service.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/scoped_user_pref_update.h" @@ -45,6 +48,11 @@ #include "third_party/abseil-cpp/absl/types/optional.h" namespace brave_news { +namespace { +// The favicon size we desire. The favicons are rendered at 24x24 pixels but +// they look quite a bit nicer if we get a 48x48 pixel icon and downscale it. +constexpr uint32_t kDesiredFaviconSizePixels = 48; +} // namespace bool IsPublisherEnabled(const mojom::Publisher* publisher) { if (!publisher) @@ -77,10 +85,12 @@ void BraveNewsController::RegisterProfilePrefs(PrefRegistrySimple* registry) { BraveNewsController::BraveNewsController( PrefService* prefs, + favicon::FaviconService* favicon_service, brave_ads::AdsService* ads_service, history::HistoryService* history_service, scoped_refptr url_loader_factory) : prefs_(prefs), + favicon_service_(favicon_service), ads_service_(ads_service), api_request_helper_(GetNetworkTrafficAnnotationTag(), url_loader_factory), private_cdn_request_helper_(GetNetworkTrafficAnnotationTag(), @@ -302,6 +312,54 @@ void BraveNewsController::GetImageData(const GURL& padded_image_url, std::move(callback), is_padded)); } +void BraveNewsController::GetFavIconData(const std::string& publisher_id, + GetFavIconDataCallback callback) { + GetPublishers(base::BindOnce( + [](BraveNewsController* controller, const std::string& publisher_id, + GetFavIconDataCallback callback, Publishers publishers) { + // If the publisher doesn't exist, there's nothing we can do. + const auto& it = publishers.find(publisher_id); + if (it == publishers.end()) { + std::move(callback).Run(absl::nullopt); + return; + } + + // If we have a FavIcon url, use that. + const auto& publisher = it->second; + if (publisher->favicon_url) { + controller->GetImageData(publisher->favicon_url.value(), + std::move(callback)); + return; + } + + auto source_url = publisher->site_url.is_valid() + ? publisher->site_url + : publisher->feed_source; + favicon_base::IconTypeSet icon_types{ + favicon_base::IconType::kFavicon, + favicon_base::IconType::kTouchIcon}; + controller->favicon_service_->GetRawFaviconForPageURL( + source_url, icon_types, kDesiredFaviconSizePixels, true, + base::BindOnce( + [](GetFavIconDataCallback callback, + const favicon_base::FaviconRawBitmapResult& result) { + if (!result.is_valid()) { + std::move(callback).Run(absl::nullopt); + return; + } + + auto bytes = result.bitmap_data; + std::vector bytes_vec( + bytes->front_as(), + bytes->front_as() + bytes->size()); + std::move(callback).Run(std::move(bytes_vec)); + }, + std::move(callback)), + &controller->task_tracker_); + }, + base::Unretained(this), publisher_id, std::move(callback))); +} + void BraveNewsController::SetPublisherPref(const std::string& publisher_id, mojom::UserEnabled new_status) { VLOG(1) << "set publisher pref: " << new_status; diff --git a/components/brave_today/browser/brave_news_controller.h b/components/brave_today/browser/brave_news_controller.h index 9afee326b7f6..03efa7daa72f 100644 --- a/components/brave_today/browser/brave_news_controller.h +++ b/components/brave_today/browser/brave_news_controller.h @@ -6,12 +6,14 @@ #ifndef BRAVE_COMPONENTS_BRAVE_TODAY_BROWSER_BRAVE_NEWS_CONTROLLER_H_ #define BRAVE_COMPONENTS_BRAVE_TODAY_BROWSER_BRAVE_NEWS_CONTROLLER_H_ +#include #include #include #include "base/callback_forward.h" #include "base/containers/flat_map.h" #include "base/memory/raw_ptr.h" +#include "base/task/cancelable_task_tracker.h" #include "base/timer/timer.h" #include "brave/components/api_request_helper/api_request_helper.h" #include "brave/components/brave_private_cdn/private_cdn_request_helper.h" @@ -37,6 +39,10 @@ namespace brave_ads { class AdsService; } // namespace brave_ads +namespace favicon { +class FaviconService; +} + namespace history { class HistoryService; } // namespace history @@ -56,6 +62,7 @@ class BraveNewsController : public KeyedService, BraveNewsController( PrefService* prefs, + favicon::FaviconService* favicon_service, brave_ads::AdsService* ads_service, history::HistoryService* history_service, scoped_refptr url_loader_factory); @@ -92,6 +99,8 @@ class BraveNewsController : public KeyedService, void RemoveDirectFeed(const std::string& publisher_id) override; void GetImageData(const GURL& padded_image_url, GetImageDataCallback callback) override; + void GetFavIconData(const std::string& publisher_id, + GetFavIconDataCallback callback) override; void SetPublisherPref(const std::string& publisher_id, mojom::UserEnabled new_status) override; void ClearPrefs() override; @@ -122,6 +131,7 @@ class BraveNewsController : public KeyedService, void Prefetch(); raw_ptr prefs_ = nullptr; + raw_ptr favicon_service_ = nullptr; raw_ptr ads_service_ = nullptr; api_request_helper::APIRequestHelper api_request_helper_; brave_private_cdn::PrivateCDNRequestHelper private_cdn_request_helper_; @@ -137,6 +147,7 @@ class BraveNewsController : public KeyedService, base::OneShotTimer timer_prefetch_; base::RepeatingTimer timer_feed_update_; base::RepeatingTimer timer_publishers_update_; + base::CancelableTaskTracker task_tracker_; mojo::ReceiverSet receivers_; base::WeakPtrFactory weak_ptr_factory_; diff --git a/components/brave_today/browser/direct_feed_controller.cc b/components/brave_today/browser/direct_feed_controller.cc index 6ceafb3d20fb..777e07beac83 100644 --- a/components/brave_today/browser/direct_feed_controller.cc +++ b/components/brave_today/browser/direct_feed_controller.cc @@ -94,19 +94,20 @@ void ParseFeedDataOffMainThread(const GURL& feed_url, base::BindOnce( [](const GURL& feed_url, const std::string& charset, std::string body_content) -> absl::optional { + std::string result; // Ensure the body is encoded as UTF-8. if (!base::ConvertToUtf8AndNormalize(body_content, charset, - &body_content)) { + &result)) { LOG(ERROR) << "Failed to convert body content of " << feed_url << " to utf-8"; return absl::nullopt; } brave_news::FeedData data; - if (!parse_feed_string(::rust::String(body_content), data)) { + if (!parse_feed_string(::rust::String(result), data)) { VLOG(1) << feed_url.spec() << " not a valid feed."; VLOG(2) << "Response body was:"; - VLOG(2) << body_content; + VLOG(2) << result; return absl::nullopt; } return data; @@ -230,7 +231,7 @@ void DirectFeedController::FindFeeds( // Response is valid, but still might not be a feed ParseFeedDataOffMainThread( - feed_url, charset, std::move(body_content), + feed_url, charset, body_content, base::BindOnce( [](const GURL& feed_url, const GURL& final_url, const std::string& mime_type, diff --git a/components/brave_today/common/brave_news.mojom b/components/brave_today/common/brave_news.mojom index 8291cf92f7fc..47989968ccc7 100644 --- a/components/brave_today/common/brave_news.mojom +++ b/components/brave_today/common/brave_news.mojom @@ -175,6 +175,7 @@ interface BraveNewsController { => (bool is_valid_feed, bool is_duplicate, map? publishers); RemoveDirectFeed(string publisher_id); GetImageData(url.mojom.Url padded_image_url) => (array? image_data); + GetFavIconData(string publisher_id) => (array? image_data); SetPublisherPref(string publisher_id, UserEnabled new_status); ClearPrefs(); IsFeedUpdateAvailable(string displayed_feed_hash) diff --git a/components/common/useVisible.ts b/components/common/useVisible.ts new file mode 100644 index 000000000000..9689aadc38e5 --- /dev/null +++ b/components/common/useVisible.ts @@ -0,0 +1,42 @@ +// Copyright (c) 2022 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. + +import * as React from 'react' + +export interface VisibleOptions { + rootElement?: HTMLElement | null + rootMargin?: string + threshold?: number +} + +export function useVisible (options: VisibleOptions) { + const [visible, setVisible] = React.useState(false) + const [elementRef, setElementRef] = React.useState(null) + + React.useEffect(() => { + if (!elementRef) return + + const observer = new IntersectionObserver(([intersectionInfo]) => { + if (!intersectionInfo.isIntersecting) { + return + } + setVisible(true) + }, { + root: options.rootElement, + rootMargin: options.rootMargin, + threshold: options.threshold + }) + + observer.observe(elementRef) + return () => { + observer.disconnect() + } + }, [options.rootElement, elementRef]) + + return { + visible, + setElementRef + } +}