-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[#65618] Remove jQuery from core #19429
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
Changes from 45 commits
38ca029
54eb1a0
9c5c58c
2b731a5
184f042
5c7ccd2
6ddd7d2
cce4b40
0e77814
d373a1b
293d93f
ca48d18
04f6449
9c0e223
bee6fd5
722f1d4
d05716a
207fbbe
435268b
c9748dd
27c6fb8
81493f9
2e879e0
9d60ac7
bd8154d
8284389
189d43f
43ea22d
53f3611
e2897d0
d2dcfd5
d130257
7fcf2dc
c2594bd
3a5a8bc
0c065cb
9a52cbc
c901941
0143e7c
8309b32
b958b5b
b9f04df
829818d
7e63b60
93f508e
dbb5534
0aeeb3d
bfac4c2
51d3486
6d190e2
37dcd13
9e0b6f5
91d2391
7cad4d2
e039c64
78834a5
cdf1586
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,11 @@ def link_to_function(content, function, html_options = {}) | |
| # Execute the callback on click | ||
| def csp_onclick(callback_str, selector, prevent_default: true) | ||
| content_for(:additional_js_dom_ready) do | ||
| "jQuery('#{selector}').click(function() { #{callback_str}; #{prevent_default ? 'return false;' : ''} });\n".html_safe | ||
| raw <<~JS # rubocop:disable Rails/OutputSafety | ||
| document.querySelector('#{selector}')?.addEventListener('click', function(event) { | ||
| #{callback_str&.delete_suffix(';')};#{"\n event.preventDefault();" if prevent_default} | ||
| }); | ||
| JS | ||
| end | ||
| end | ||
|
Comment on lines
46
to
54
|
||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,28 +27,6 @@ See COPYRIGHT and LICENSE files for more details. | |
|
|
||
| ++#%> | ||
|
|
||
| <div id="add-message" style="display:none;"> | ||
| <% if authorize_for('messages', 'new') %> | ||
| <h2><%= link_to h(@forum.name), project_forum_path(@project, @forum) %> » <%= t(:label_message_new) %></h2> | ||
| <%= labelled_tabular_form_for Message.new(project: @project), | ||
| url: forum_topics_path(@forum), | ||
| html: { | ||
| multipart: true, | ||
| id: "message-form", | ||
| class: "form", | ||
| data: { turbo: false } | ||
| } do |f| %> | ||
| <%= render partial: "messages/form", locals: { f: f } %> | ||
|
|
||
| <hr class="form--separator"> | ||
| <%= styled_button_tag t(:button_create), class: "-primary -with-icon icon-checkmark" %> | ||
| <%= link_to t(:button_cancel), "", class: "cancel-add-message-button button -with-icon icon-cancel" %> | ||
| <% csp_onclick('jQuery("#add-message").hide();', ".cancel-add-message-button") %> | ||
| <% end %> | ||
| <div id="preview"></div> | ||
| <% end %> | ||
| </div> | ||
|
Comment on lines
-30
to
-50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| <%= | ||
| render(Primer::OpenProject::PageHeader.new) do |header| | ||
| header.with_title { @forum.name } | ||
|
|
@@ -70,7 +48,6 @@ See COPYRIGHT and LICENSE files for more details. | |
| leading_icon: :plus, | ||
| label: t(:label_message_new), | ||
| tag: :a, | ||
| class: "add-message-button", | ||
| href: url_for({ controller: "/messages", action: "new", forum_id: @forum })) do | ||
| t(:label_message) | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,6 @@ See COPYRIGHT and LICENSE files for more details. | |
| aria: { label: I18n.t(:button_edit) }, | ||
| title: I18n.t(:button_edit) | ||
| ) do |button| | ||
| csp_onclick('jQuery("#edit-news").show()', ".edit-news-button") | ||
| button.with_leading_visual_icon(icon: :pencil) | ||
| t(:button_edit) | ||
| end | ||
|
|
@@ -72,16 +71,6 @@ See COPYRIGHT and LICENSE files for more details. | |
| end | ||
| %> | ||
|
|
||
| <% if authorize_for('news', 'edit') %> | ||
| <div id="edit-news" style="display:none;"> | ||
| <%= labelled_tabular_form_for @news, html: { id: "news-form" } do |f| %> | ||
| <%= render partial: "form", locals: { f: f } %> | ||
| <%= styled_button_tag t(:button_save), class: "-primary -with-icon icon-checkmark" %> | ||
| <%= link_to_function t(:button_cancel), 'jQuery("#edit-news").hide()', | ||
| class: "button -with-icon icon-cancel" %> | ||
| <% end %> | ||
| </div> | ||
| <% end %> | ||
|
Comment on lines
-75
to
-84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| <% if @news.summary.present? %> | ||
| <div class="summary"> | ||
| <%= format_text(@news, :summary) %> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,10 +67,16 @@ See COPYRIGHT and LICENSE files for more details. | |
| ) %> | ||
| <%= | ||
| content_for(:additional_js_dom_ready) do | ||
| "jQuery('#repository-password-placeholder') | ||
| .change(function() { this.name = 'repository[password]'; }) | ||
| .focus(function() { this.value = ''; this.name = 'repository[password]'; }); | ||
| ".html_safe | ||
| raw <<~JS | ||
| const placeholder = document.getElementById('repository-password-placeholder'); | ||
| placeholder?.addEventListener('input', function(event) { | ||
| placeholder.name = 'repository[password]'; | ||
| }); | ||
| placeholder?.addEventListener('focus', function(event) { | ||
| placeholder.value = ''; | ||
| placeholder.name = 'repository[password]'; | ||
| }); | ||
| JS | ||
|
Comment on lines
+70
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ported this over, but don't really understand why we need to do this. |
||
| end | ||
| %> | ||
| </div> | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
csp_onclickmethod is vulnerable to XSS injection. Theselectorandcallback_strparameters are directly interpolated into JavaScript without escaping. If user-controlled data reaches these parameters, malicious JavaScript could be executed. Consider using proper JavaScript escaping or a safer templating approach.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliverguenther I think we can live with this. User-generated data should never be passed to this method.