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.

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

[JENKINS-74832] disable copybutton in insecure context #10141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Jan 9, 2025

Copying to clipboard only works in a secure context (https). So instead of only showing that it doesn't work after pressing the button, the button is now disabled and a corresponding tooltip is shown.

After:
image

See JENKINS-74832.

Testing done

  • Copy button is disabled as expected when in an insecure context (http url), e.g. on console log of a job

Proposed changelog entries

  • Disable copy button in insecure contexts.

Proposed upgrade guidelines

N/A

Submitter checklist

Preview Give feedback

Desired reviewers

N/A

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

Copying to clipboard only works in a secure context (https). So instead
of only showing that it doesn't work after pressing the button, the
button is now disabled and a corresponding tooltip is shown.
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

I tested interactively in the agent startup page and in the script console output page with an insecure connection and it behaved as expected. Thanks!

@MarkEWaite MarkEWaite added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Jan 9, 2025
Comment on lines +34 to +35
const parent = copyButton.parentElement;
parent.setAttribute("tooltip", parent.dataset.messageInsecure);
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why add the span when you could put the data- attribute on the button itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tooltip attribute of a button is not working with tippy when the button is disabled. It will only show the browser built-in tooltip. So I need the span around the button to be able to show a tippy tooltip anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants