From 1f20883ae4150db172900ae9ed357bd5623fa89d Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 12:47:17 +0100 Subject: [PATCH 1/8] Use external javascript for check-all checkbox handler --- app/assets/javascripts/publify_admin.js | 1 + app/views/admin/feedback/article.html.erb | 2 +- app/views/admin/feedback/index.html.erb | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/publify_admin.js b/app/assets/javascripts/publify_admin.js index 0cff60aa..fcc35980 100644 --- a/app/assets/javascripts/publify_admin.js +++ b/app/assets/javascripts/publify_admin.js @@ -67,6 +67,7 @@ $(document).ready(function() { $('#article_form').each(function(e){autosave_request(e)}); $('#article_form').submit(function(e){save_article_tags()}); $('#article_form').each(function(e){tag_manager()}); + $('#checkall').click(function(e){check_all(e.target)}); // DropDown $(".dropdown-toggle").dropdown(); diff --git a/app/views/admin/feedback/article.html.erb b/app/views/admin/feedback/article.html.erb index eba43aa3..165275d1 100644 --- a/app/views/admin/feedback/article.html.erb +++ b/app/views/admin/feedback/article.html.erb @@ -16,7 +16,7 @@ - + <%= t(".author") %> <%= t(".created_at") %> diff --git a/app/views/admin/feedback/index.html.erb b/app/views/admin/feedback/index.html.erb index a40df3c9..e29a353a 100644 --- a/app/views/admin/feedback/index.html.erb +++ b/app/views/admin/feedback/index.html.erb @@ -27,7 +27,7 @@ - + <%= t(".author") %> <%= t(".created_at") %> @@ -52,4 +52,4 @@ <% end %> -
+
From cc3acb1111a00fc881b13351a83b1067891ce5c5 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 13:41:44 +0100 Subject: [PATCH 2/8] Re-implement Markup Help dialog using external JavaScript --- app/assets/javascripts/markup_help_popup.js | 15 +++++++++++++++ app/assets/javascripts/publify.js | 1 + app/assets/stylesheets/publify.css.scss | 13 +++++++++++++ app/helpers/base_helper.rb | 15 ++++++++++++--- 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 app/assets/javascripts/markup_help_popup.js diff --git a/app/assets/javascripts/markup_help_popup.js b/app/assets/javascripts/markup_help_popup.js new file mode 100644 index 00000000..c07c5c49 --- /dev/null +++ b/app/assets/javascripts/markup_help_popup.js @@ -0,0 +1,15 @@ +$(document).ready(function() { + $('.markup-help-popup-link').on("click", function(e){ + var dialog = document.getElementById(e.target.dataset["target"]); + dialog.showModal(); + e.preventDefault(); + }); + $('.markup-help-popup-close').on("click", function(e) { + e.target.closest('dialog').close(); + }); + $('.markup-help-popup').on("click", function(e) { + if (e.target == e.currentTarget) { + e.target.close(); + } + }); +}); diff --git a/app/assets/javascripts/publify.js b/app/assets/javascripts/publify.js index dbf9bb56..e8b10219 100644 --- a/app/assets/javascripts/publify.js +++ b/app/assets/javascripts/publify.js @@ -6,6 +6,7 @@ //= require set-timeago-lang //= require jquery_ujs //= require lightbox +//= require markup_help_popup //= require observe //= require check_password // diff --git a/app/assets/stylesheets/publify.css.scss b/app/assets/stylesheets/publify.css.scss index c250348f..55125275 100644 --- a/app/assets/stylesheets/publify.css.scss +++ b/app/assets/stylesheets/publify.css.scss @@ -7,3 +7,16 @@ border-bottom: #eee 1px solid; font-size: 0.9em; } + +.markup-help-popup { + padding: 0; +} + +.markup-help-popup > div { + padding: 1em; +} + +.markup-help-popup-close { + float: right; + cursor: pointer; +} diff --git a/app/helpers/base_helper.rb b/app/helpers/base_helper.rb index 8fe6c982..6520b069 100644 --- a/app/helpers/base_helper.rb +++ b/app/helpers/base_helper.rb @@ -71,9 +71,18 @@ def meta_tag(name, value) def markup_help_popup(markup, text) if markup && markup.commenthelp.size > 1 - link_to(text, - url_for(controller: "articles", action: "markup_help", id: markup.name), - onclick: "return popup(this, 'Publify Markup Help')") + modal = tag.dialog id: "this_markup_help_popup_dialog", class: "markup-help-popup" do + tag.div do + close_div = tag.div tag.span("\u2a09", class: "markup-help-popup-close") + content = sanitize(markup.commenthelp) + safe_join [close_div, content] + end + end + + link = link_to(text, "#", class: "markup-help-popup-link", + data: { target: "this_markup_help_popup_dialog" }) + + safe_join [modal, link] else "" end From cd2886d9b93201aa293ace2f058d1114d78d923c Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 15:41:04 +0100 Subject: [PATCH 3/8] Implement optional field toggle with external javascript --- app/assets/javascripts/optional_field_toggle.js | 6 ++++++ app/assets/javascripts/publify.js | 1 + app/views/articles/_comment_form.html.erb | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/optional_field_toggle.js diff --git a/app/assets/javascripts/optional_field_toggle.js b/app/assets/javascripts/optional_field_toggle.js new file mode 100644 index 00000000..f6b85516 --- /dev/null +++ b/app/assets/javascripts/optional_field_toggle.js @@ -0,0 +1,6 @@ +$(document).ready(function() { + $('.optional-field-toggle').on("click", function(e){ + $('.optional_field').fadeToggle(); + e.preventDefault(); + }); +}); diff --git a/app/assets/javascripts/publify.js b/app/assets/javascripts/publify.js index e8b10219..e6d4bd67 100644 --- a/app/assets/javascripts/publify.js +++ b/app/assets/javascripts/publify.js @@ -8,6 +8,7 @@ //= require lightbox //= require markup_help_popup //= require observe +//= require optional_field_toggle //= require check_password // //= require_self diff --git a/app/views/articles/_comment_form.html.erb b/app/views/articles/_comment_form.html.erb index 91b111a2..1b554d87 100644 --- a/app/views/articles/_comment_form.html.erb +++ b/app/views/articles/_comment_form.html.erb @@ -10,7 +10,7 @@ <%= text_field 'comment', 'author', size: 20 %> - <%= link_to '#', onclick: "$('.optional_field').fadeToggle();return false" do %> + <%= link_to '#', class: "optional-field-toggle" do %> (<%= t('.leave_url_email') %> ») <% end %> From e750d568c11e2e0b87bdd6cc5a25cee8476bf729 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 16:05:11 +0100 Subject: [PATCH 4/8] Use external javascript for comment preview link --- app/assets/javascripts/preview_comment.js | 10 ++++++++++ app/assets/javascripts/publify.js | 1 + app/views/articles/_comment_form.html.erb | 6 ++++-- 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/preview_comment.js diff --git a/app/assets/javascripts/preview_comment.js b/app/assets/javascripts/preview_comment.js new file mode 100644 index 00000000..5372bb6b --- /dev/null +++ b/app/assets/javascripts/preview_comment.js @@ -0,0 +1,10 @@ +$(document).ready(function() { + $('.preview-comment-link').on("click", function(e) { + var lnk = e.currentTarget; + var preview_url = lnk.dataset.previewUrl; + var comment_form_selector = lnk.dataset.targetForm; + + $.post(preview_url, $(comment_form_selector).serialize()); + e.preventDefault(); + }); +}); diff --git a/app/assets/javascripts/publify.js b/app/assets/javascripts/publify.js index e6d4bd67..48447384 100644 --- a/app/assets/javascripts/publify.js +++ b/app/assets/javascripts/publify.js @@ -9,6 +9,7 @@ //= require markup_help_popup //= require observe //= require optional_field_toggle +//= require preview_comment //= require check_password // //= require_self diff --git a/app/views/articles/_comment_form.html.erb b/app/views/articles/_comment_form.html.erb index 1b554d87..25ad844d 100644 --- a/app/views/articles/_comment_form.html.erb +++ b/app/views/articles/_comment_form.html.erb @@ -39,8 +39,10 @@ <%= markup_help_popup TextFilter.make_filter(this_blog.comment_text_filter), t('.comment_markup_help') %> - <%= t('.preview_comment') %> - + <%= link_to "#", data: { preview_url: @article.preview_comment_url, target_form: "#comment_form" }, class: "preview-comment-link" do %> + <%= t('.preview_comment') %> + <% end %> + From 6d997fb334faffab533d4c55abbe48e5b87410f1 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 17:08:14 +0100 Subject: [PATCH 5/8] Re-implement admin hover functionality without cookies and javascript The cookie that this functionality was relying on has not been set since 2016. This re-implements the function in the backend. This means the resulting HTML should not be cached. For comments and trackbacks, the helper call is simply removed, since the edit link was also missing. To avoid breaking any themes that rely on it, the helper itself is just made a no-op. --- app/assets/stylesheets/publify.css.scss | 8 ++++++++ app/helpers/base_helper.rb | 10 +++------- app/views/articles/_trackback.html.erb | 6 +++--- app/views/articles/read.html.erb | 6 ++++-- app/views/comments/_comment.html.erb | 2 +- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/publify.css.scss b/app/assets/stylesheets/publify.css.scss index 55125275..b7270ecd 100644 --- a/app/assets/stylesheets/publify.css.scss +++ b/app/assets/stylesheets/publify.css.scss @@ -20,3 +20,11 @@ float: right; cursor: pointer; } + +.admintools { + display: none; +} + +.admin-tools-reveal:hover .admintools { + display: block; +} diff --git a/app/helpers/base_helper.rb b/app/helpers/base_helper.rb index 6520b069..c76e0d50 100644 --- a/app/helpers/base_helper.rb +++ b/app/helpers/base_helper.rb @@ -88,13 +88,9 @@ def markup_help_popup(markup, text) end end - def onhover_show_admin_tools(type, id = nil) - admin_id = "#admin_#{[type, id].compact.join("_")}" - tag = [] - tag << %{ onmouseover="if (getCookie('publify_user_profile') == 'admin')\ - { $('#{admin_id}').show(); }" } - tag << %{ onmouseout="$('#{admin_id}').hide();" } - safe_join(tag, " ") + # This method's original implementation was broken. Now it does nothing. + def onhover_show_admin_tools(_type, _id = nil) + "" end def feed_title diff --git a/app/views/articles/_trackback.html.erb b/app/views/articles/_trackback.html.erb index cdb31f62..44dbe2f5 100644 --- a/app/views/articles/_trackback.html.erb +++ b/app/views/articles/_trackback.html.erb @@ -1,6 +1,6 @@ -
  • > +
  • - <%= t('.from') %> <%= trackback.blog_name %>
    - <%= h trackback.title %>
    + <%= t('.from') %> <%= trackback.blog_name %>
    + <%= h trackback.title %>
    <%= trackback.excerpt %>
    • diff --git a/app/views/articles/read.html.erb b/app/views/articles/read.html.erb index 7880045d..8ecab910 100644 --- a/app/views/articles/read.html.erb +++ b/app/views/articles/read.html.erb @@ -1,5 +1,7 @@ -
    > - <%= link_to(t('.edit'), { controller: 'admin/articles', action: 'edit', id: @article.id }, { class: 'admintools', style: 'display: none', id: 'admin_article' }) %> +
    + <% if current_user&.profile == "admin" %> + <%= link_to(t('.edit'), edit_admin_article_path(@article.id), class: 'admintools', id: 'admin_article') %> + <% end %> <% cache @article do %>

    <%= link_to_permalink @article, @article.title %>

    <%= render 'articles/article_author', article: @article %> diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 7f01dcc8..97c2cb73 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,5 +1,5 @@ <% cache comment do %> -
  • <%= onhover_show_admin_tools(:comment, comment.id) %>> +
  • >

    <%= avatar_tag(email: comment.email, url: comment.url) %> <%= link_to_unless(comment.url.blank?, h(comment.author), comment.url) %> From 873f5db0c16f968afe982012b7710352b167b84f Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 18:26:44 +0100 Subject: [PATCH 6/8] Load markup help only when requested This avoids having large explanatory text in the base article HTML, and avoids triggering the check for doubly-escaped HTML in the specs. The latter issue is due to the fact that the markdown helper text contains escaped HTML in its examples. --- app/assets/javascripts/markup_help_popup.js | 12 +++++++++++- app/controllers/articles_controller.rb | 4 +++- app/helpers/base_helper.rb | 7 +++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/markup_help_popup.js b/app/assets/javascripts/markup_help_popup.js index c07c5c49..e52a548a 100644 --- a/app/assets/javascripts/markup_help_popup.js +++ b/app/assets/javascripts/markup_help_popup.js @@ -1,7 +1,17 @@ $(document).ready(function() { $('.markup-help-popup-link').on("click", function(e){ var dialog = document.getElementById(e.target.dataset["target"]); - dialog.showModal(); + var url = e.target.dataset.url; + + $.ajax({ + url: url, + type: 'get', + dataType: 'html', + success: function(data) { + dialog.getElementsByClassName("content-target").item(0).innerHTML = data; + dialog.showModal(); + } + }); e.preventDefault(); }); $('.markup-help-popup-close').on("click", function(e) { diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index f32ec29a..4aa6c463 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ArticlesController < ContentController + include ActionView::Helpers::SanitizeHelper + before_action :login_required, only: [:preview, :preview_page] before_action :verify_config before_action :auto_discovery_feed, only: [:show, :index] @@ -127,7 +129,7 @@ def view_page def markup_help filter = TextFilter.make_filter(params[:id]) if filter - render html: filter.commenthelp + render html: sanitize(filter.commenthelp) else render plain: "Unknown filter" end diff --git a/app/helpers/base_helper.rb b/app/helpers/base_helper.rb index c76e0d50..1ffb3756 100644 --- a/app/helpers/base_helper.rb +++ b/app/helpers/base_helper.rb @@ -74,13 +74,16 @@ def markup_help_popup(markup, text) modal = tag.dialog id: "this_markup_help_popup_dialog", class: "markup-help-popup" do tag.div do close_div = tag.div tag.span("\u2a09", class: "markup-help-popup-close") - content = sanitize(markup.commenthelp) + content = tag.div class: "content-target" safe_join [close_div, content] end end + url = url_for(controller: "articles", action: "markup_help", id: markup.name) + link = link_to(text, "#", class: "markup-help-popup-link", - data: { target: "this_markup_help_popup_dialog" }) + data: { target: "this_markup_help_popup_dialog", + url: url }) safe_join [modal, link] else From f101123ff28670d6bf9edf5a2320611580954a90 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 18:29:22 +0100 Subject: [PATCH 7/8] Regenerate RuboCop to-do file --- .rubocop_todo.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cefadc4d..945cca20 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --no-offense-counts --no-auto-gen-timestamp` -# using RuboCop version 1.56.4. +# using RuboCop version 1.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -35,7 +35,7 @@ Metrics/MethodLength: # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 209 + Max: 213 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: From e433b0228957dd0bf83ed494eade3dff0935aa61 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 5 Nov 2023 18:31:36 +0100 Subject: [PATCH 8/8] Update the manifest --- Manifest.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Manifest.txt b/Manifest.txt index 260cf363..d217d0d1 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -37,7 +37,10 @@ app/assets/javascripts/lang/fr_FR.js app/assets/javascripts/lang/nl_NL.js app/assets/javascripts/lang/zh_TW.js app/assets/javascripts/lightbox.js +app/assets/javascripts/markup_help_popup.js app/assets/javascripts/observe.js +app/assets/javascripts/optional_field_toggle.js +app/assets/javascripts/preview_comment.js app/assets/javascripts/publify.js app/assets/javascripts/publify_admin.js app/assets/javascripts/quicktags.js