Skip to content

Migrate inline JavaScript event handlers#412

Merged
vagisha merged 4 commits into
release24.3-SNAPSHOTfrom
24.3_fb_migrate-inline-JS-handlers
Mar 28, 2024
Merged

Migrate inline JavaScript event handlers#412
vagisha merged 4 commits into
release24.3-SNAPSHOTfrom
24.3_fb_migrate-inline-JS-handlers

Conversation

@vagisha
Copy link
Copy Markdown
Collaborator

@vagisha vagisha commented Mar 28, 2024

Changes

Migrated inline even handlers in the panoramapublic and lincs modules

  • CustomGCT.html
  • customGCTForm.jsp
  • ExperimentAnnotationsTableInfo.java
  • experimentDetails.jsp
  • pxValidationStatus.jsp
  • panoramaWebSearch.jsp
  • viewSlideshow.jsp
  • slideshow.js

@vagisha vagisha requested a review from labkey-jeckels March 28, 2024 03:51
var link = new Link.LinkBuilder("Share")
.clearClasses().addClass("button-small button-small-green")
.style("margin:0px 5px 0px 2px;")
.onClick("showShareLink(this, '" + PageFlowUtil.filter(accessUrl) + "');return false;");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be OK in practice, but could break if the value being rendered includes single quotes.

Suggested change
.onClick("showShareLink(this, '" + PageFlowUtil.filter(accessUrl) + "');return false;");
.onClick("showShareLink(this, " + PageFlowUtil.jsString(accessUrl) + ");return false;");

<a class="button-small button-small-green" style="margin:0px 5px 0px 2px;" href="" onclick="showShareLink(this, '<%=h(accessUrl)%>'); return false;">Share</a>
<%=link("Share").clearClasses().addClass("button-small button-small-green")
.style("margin:0px 5px 0px 2px")
.onClick("showShareLink(this, '" + h(accessUrl) + "'); return false;")%>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.onClick("showShareLink(this, '" + h(accessUrl) + "'); return false;")%>
.onClick("showShareLink(this, " + q(accessUrl) + "); return false;")%>

@vagisha vagisha merged commit c449767 into release24.3-SNAPSHOT Mar 28, 2024
@vagisha vagisha deleted the 24.3_fb_migrate-inline-JS-handlers branch March 28, 2024 21:29
}
LABKEY.Utils.onReady(function()
{
document.getElementById("customGctButton").onclick = function() { showCustomGCTForm(); };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is already merged and no need to change, but:

  • By convention, we usually do something like: document.getElementById("customGctButton")['onclick'] = function().... This drops the migrated handlers from text searches looking for typical inline handler patterns ("onclick = ").
  • Since there are no parameters, = showCustomGCTForm; should work as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants