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.

Sign up for GitHub

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

Jump to bottom

Confirmation before connecting to youtube.com/youtu.be #4826

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Confirmation before connecting to youtube.com/youtu.be #4826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a02b55b
feat: add confirm leaving Invidious view and route
unlxam Aug 9, 2024
f7e5a88
feat: update youtube.com links to confirm before leaving
unlxam Aug 10, 2024
e851144
fix: remove redundant Invidious from title
unlxam Aug 10, 2024
ecfb9fe
feat: replace external links in description and comments to confirm b…
unlxam Aug 10, 2024
b240f91
feat: add link url
unlxam Aug 10, 2024
01e9fa3
feat: add rel="noreferrer noopener" to external link
unlxam Aug 24, 2024
0fd38e5
refactor: replace all youtube external links with confirmation page
unlxam Aug 24, 2024
effb642
feat: replace View channel on YouTube and channel's description exter…
unlxam Aug 24, 2024
42ac3c8
feat: add confirmation page text
unlxam Aug 24, 2024
be1ed83
feat: add presence checking and html escaping for link
unlxam Aug 24, 2024
307e6f5
refactor: add better link presence checking
unlxam Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion locales/en-US.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,5 +496,6 @@
"toggle_theme": "Toggle Theme",
"carousel_slide": "Slide {{current}} of {{total}}",
"carousel_skip": "Skip the Carousel",
"carousel_go_to": "Go to slide `x`"
"carousel_go_to": "Go to slide `x`",
"confirm_dialog_external_link": "You are leaving Invidious. Continue to external link?"
}
3 changes: 3 additions & 0 deletions src/invidious/channels/about.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ def get_about_info(ucid, locale) : AboutChannel
end
end

# use comments link_utils to replace external links with the confirmation page
description_html = Invidious::Comments.replace_external_links(description_html)

sub_count = 0

if (metadata_rows = initdata.dig?("header", "pageHeaderRenderer", "content", "pageHeaderViewModel", "metadata", "contentMetadataViewModel", "metadataRows").try &.as_a)
Expand Down
26 changes: 26 additions & 0 deletions src/invidious/comments/links_util.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,30 @@ module Invidious::Comments

return html.to_xml(options: XML::SaveOptions::NO_DECL)
end

def replace_external_links(html)
# Check if the document is empty
# Prevents edge-case bug with Reddit comments, see issue #3115
if html.nil? || html.empty?
return html
end

html = XML.parse_html(html)

html.xpath_nodes(%q(//a)).each do |anchor|
url = URI.parse(anchor["href"])

if !url.host.nil? && !url.host.not_nil!.ends_with?("youtube.com") && !url.host.not_nil!.ends_with?("youtu.be")
confirm_leave = "/confirm_leave?link=#{URI.encode_path(url.to_s)}"
anchor["href"] = confirm_leave
end
end

html = html.xpath_node(%q(//body)).not_nil!
if node = html.xpath_node(%q(./p))
html = node
end

return html.to_xml(options: XML::SaveOptions::NO_DECL)
end
end
1 change: 1 addition & 0 deletions src/invidious/comments/youtube.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ module Invidious::Comments
if format == "html"
response = JSON.parse(response)
content_html = Frontend::Comments.template_youtube(response, locale, thin_mode)
content_html = Comments.replace_external_links(content_html)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you added this here? Is it because you missed one link to replace, as mentioned here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for replacing the links on comments with the confirmation page, such as when creators pin their own with links from Discord or Twitch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, okay, I see!

response = JSON.build do |json|
json.object do
json.field "contentHtml", content_html
Expand Down
10 changes: 8 additions & 2 deletions src/invidious/frontend/comments_youtube.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "uri"

module Invidious::Frontend::Comments
extend self

Expand Down Expand Up @@ -148,13 +150,17 @@ module Invidious::Frontend::Comments
END_HTML

if comments["videoId"]?
permalink = "https://www.youtube.com/watch?v=#{comments["videoId"]}&lc=#{child["commentId"]}"
permalink_confirm = "/confirm_leave?link=#{URI.encode_path(permalink)}"
html << <<-END_HTML
<a rel="noreferrer noopener" href="https://www.youtube.com/watch?v=#{comments["videoId"]}&lc=#{child["commentId"]}" title="#{translate(locale, "YouTube comment permalink")}">[YT]</a>
<a href="#{permalink_confirm}" title="#{translate(locale, "YouTube comment permalink")}">[YT]</a>
unlxam marked this conversation as resolved.
Show resolved Hide resolved
|
END_HTML
elsif comments["authorId"]?
permalink = "https://www.youtube.com/channel/#{comments["authorId"]}/community?lb=#{child["commentId"]}"
permalink_confirm = "/confirm_leave?link=#{URI.encode_path(permalink)}"
html << <<-END_HTML
<a rel="noreferrer noopener" href="https://www.youtube.com/channel/#{comments["authorId"]}/community?lb=#{child["commentId"]}" title="#{translate(locale, "YouTube comment permalink")}">[YT]</a>
<a href="#{permalink_confirm}" title="#{translate(locale, "YouTube comment permalink")}">[YT]</a>
|
END_HTML
end
Expand Down
5 changes: 4 additions & 1 deletion src/invidious/helpers/errors.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ def error_redirect_helper(env : HTTP::Server::Context)
go_to_youtube = translate(locale, "next_steps_error_message_go_to_youtube")
switch_instance = translate(locale, "Switch Invidious Instance")

youtube_link = "https://youtube.com#{env.request.resource}"
youtube_link_confirm = "/confirm_leave?link=#{URI.encode_path(youtube_link)}"

return <<-END_HTML
<p style="margin-bottom: 4px;">#{next_steps_text}</p>
<ul>
Expand All @@ -190,7 +193,7 @@ def error_redirect_helper(env : HTTP::Server::Context)
<a href="/redirect?referer=#{env.get("current_page")}">#{switch_instance}</a>
</li>
<li>
<a rel="noreferrer noopener" href="https://youtube.com#{env.request.resource}">#{go_to_youtube}</a>
<a href="#{youtube_link_confirm}">#{go_to_youtube}</a>
</li>
</ul>
END_HTML
Expand Down
13 changes: 13 additions & 0 deletions src/invidious/routes/misc.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,17 @@ module Invidious::Routes::Misc
instance_url = fetch_random_instance
env.redirect "https://#{instance_url}#{referer}"
end

def self.confirm_leave(env)
locale = env.get("preferences").as(Preferences).locale
referer = get_referer(env)

if env.params.query["link"]? && !env.params.query["link"].empty?
link = HTML.escape(env.params.query["link"].to_s)

templated "confirm_leave"
else
env.redirect "#{referer}"
end
Comment on lines +51 to +57
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small improvements:

  • .presence will return nil if env.params.query["link"]? is either nil (= the parameter is absent) or an empty string. Otherwise, it returns its value.
  • you can assign a variable inside an if condition. The if will check if that variable is truthy or falsey.
  • For the referer, the string interpolation is superfluous
  • There was a trailing whitespace on the empty line after link = .... Don't forget to run make format to ensure that your code is properly formatted!
Suggested change
if env.params.query["link"]? && !env.params.query["link"].empty?
link = HTML.escape(env.params.query["link"].to_s)
templated "confirm_leave"
else
env.redirect "#{referer}"
end
if raw_link = env.params.query["link"]?.presence
link = HTML.escape(raw_link)
templated "confirm_leave"
else
env.redirect referer
end

end
end
1 change: 1 addition & 0 deletions src/invidious/routing.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module Invidious::Routing
get "/privacy", Routes::Misc, :privacy
get "/licenses", Routes::Misc, :licenses
get "/redirect", Routes::Misc, :cross_instance_redirect
get "/confirm_leave", Routes::Misc, :confirm_leave

self.register_channel_routes
self.register_watch_routes
Expand Down
3 changes: 3 additions & 0 deletions src/invidious/videos/parser.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@

description_html = parse_description(video_secondary_renderer.try &.dig?("attributedDescription"), video_id)

# use comments link_utils to replace external links with the confirmation page
description_html = Invidious::Comments.replace_external_links(description_html)

Check failure on line 320 in src/invidious/videos/parser.cr

View workflow job for this annotation

GitHub Actions / build - crystal: 1.9.2, stable: true 

undefined constant Invidious::Comments

Check failure on line 320 in src/invidious/videos/parser.cr

View workflow job for this annotation

GitHub Actions / build - crystal: 1.10.1, stable: true 

undefined constant Invidious::Comments

Check failure on line 320 in src/invidious/videos/parser.cr

View workflow job for this annotation

GitHub Actions / build - crystal: 1.11.2, stable: true 

undefined constant Invidious::Comments

Check failure on line 320 in src/invidious/videos/parser.cr

View workflow job for this annotation

GitHub Actions / build - crystal: 1.12.1, stable: true 

undefined constant Invidious::Comments

Check failure on line 320 in src/invidious/videos/parser.cr

View workflow job for this annotation

GitHub Actions / build - crystal: nightly, stable: false 

undefined constant Invidious::Comments

# Video metadata

metadata = video_secondary_renderer
Expand Down
5 changes: 4 additions & 1 deletion src/invidious/views/components/channel_info.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
<div class="pure-g h-box">
<div class="pure-u-1-2">
<div class="pure-u-1 pure-md-1-3">
<a href="<%= youtube_url %>"><%= translate(locale, "View channel on YouTube") %></a>
<%-
confirm_view_yt = "/confirm_leave?link=#{URI.encode_path(youtube_url)}"
-%>
<a href="<%= confirm_view_yt %>"><%= translate(locale, "View channel on YouTube") %></a>
</div>
<div class="pure-u-1 pure-md-1-3">
<a href="<%= redirect_url %>"><%= translate(locale, "Switch Invidious Instance") %></a>
Expand Down
6 changes: 5 additions & 1 deletion src/invidious/views/components/video-context-buttons.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<div class="flex-right flexible">
<div class="icon-buttons">
<a title="<%=translate(locale, "videoinfo_watch_on_youTube")%>" rel="noreferrer noopener" href="https://www.youtube.com/watch<%=endpoint_params%>">
<%-
watch_yt = "https://www.youtube.com/watch#{endpoint_params}"
confirm_watch_yt = "/confirm_leave?link=#{URI.encode_path(watch_yt)}"
-%>
<a title="<%=translate(locale, "videoinfo_watch_on_youTube")%>" href="<%= confirm_watch_yt %>">
<i class="icon ion-logo-youtube"></i>
</a>
<a title="<%=translate(locale, "Audio mode")%>" href="/watch<%=endpoint_params%>&listen=1">
Expand Down
22 changes: 22 additions & 0 deletions src/invidious/views/confirm_leave.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<% content_for "header" do %>
<title><%= translate(locale, "Leave") %> - Invidious</title>
<% end %>

<div class="h-box">
<h3><%= translate(locale, "You are leaving Invidious. Continue to external link?") %></h3>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add this line to locales/en-US.json, otherwise Weblate won't be able to detect it, and in turn that won't be proposed to translators:

"confirm_dialog_external_link": "You are leaving Invidious. Continue to external link?"
Suggested change
<h3><%= translate(locale, "You are leaving Invidious. Continue to external link?") %></h3>
<h3><%= translate(locale, "confirm_dialog_external_link") %></h3>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the suggestion I made above! You need to use confirm_dialog_external_link, not the translated string here!


<p><%= link %></p>

<div class="pure-g">
<div class="pure-u-1-2">
<a rel="noreferrer noopener" value="confirm_leave" class="pure-button pure-button-primary" href="<%= link %>">
<%= translate(locale, "Yes") %>
</a>
</div>
<div class="pure-u-1-2">
<a class="pure-button" href="<%= referer %>">
<%= translate(locale, "No") %>
</a>
</div>
</div>
</div>
6 changes: 5 additions & 1 deletion src/invidious/views/playlist.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@

<% if !playlist.is_a? InvidiousPlaylist %>
<div class="pure-u-2-3">
<a rel="noreferrer noopener" href="https://www.youtube.com/playlist?list=<%= playlist.id %>">
<%-
view_yt = "https://www.youtube.com/playlist?list=#{playlist.id}"
confirm_view_yt = "/confirm_leave?link=#{URI.encode_path(view_yt)}"
-%>
<a href="<%= confirm_view_yt %>">
<%= translate(locale, "View playlist on YouTube") %>
</a>
<span> | </span>
Expand Down
7 changes: 5 additions & 2 deletions src/invidious/views/watch.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,12 @@ we're going to need to do it here in order to allow for translations.
link_yt_watch = IV::HttpServer::Utils.add_params_to_url(link_yt_watch, link_yt_param)
link_yt_embed = IV::HttpServer::Utils.add_params_to_url(link_yt_embed, link_yt_param)
end

confirm_yt_watch = "/confirm_leave?link=#{URI.encode_path(link_yt_watch.to_s)}"
confirm_yt_embed = "/confirm_leave?link=#{URI.encode_path(link_yt_embed.to_s)}"
-%>
<a id="link-yt-watch" rel="noreferrer noopener" data-base-url="<%= link_yt_watch %>" href="<%= link_yt_watch %>"><%= translate(locale, "videoinfo_watch_on_youTube") %></a>
(<a id="link-yt-embed" rel="noreferrer noopener" data-base-url="<%= link_yt_embed %>" href="<%= link_yt_embed %>"><%= translate(locale, "videoinfo_youTube_embed_link") %></a>)
<a id="link-yt-watch" data-base-url="<%= confirm_yt_watch %>" href="<%= confirm_yt_watch %>"><%= translate(locale, "videoinfo_watch_on_youTube") %></a>
(<a id="link-yt-embed" data-base-url="<%= confirm_yt_embed %>" href="<%= confirm_yt_embed %>"><%= translate(locale, "videoinfo_youTube_embed_link") %></a>)
</span>

<p id="watch-on-another-invidious-instance">
Expand Down
Loading