Highlight insecure element and scroll to it #73

cadic · 2022-05-24T14:44:15Z

Description of the Change

Adds "View element" button, which scrolls editor to the element with insecure content. The element is highlighted with the red outline.

Works both in Classic and Block editor.

The outline is cleared before every new content check, post save or preview.

Closes #9

Verification Process

Create new post
Add some content and put insecure element/block at the end of the content (outside of the first screen)
Try to save the post, the insecure warning should appear
Click on "View element" link/button (depends on the Block or Classic editor you use)
The page expected to scroll to the element/block and add a red outline to it

highlight-scroll.mp4

Possible Drawbacks

The original tracking design doesn't allow to identify exact element/block which refers to insecure content. We only have list of URLs, which is used in this PR to find elements in the editor.

If the content have multiple elements with the same URL, multiple items with this URL will be added to the warning block. But any "View element" link from the identical URLs will only scroll/highlight the first element.

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests passed.

Changelog Entry

Added - "View element" link to highlight and auto-scroll to the insecure element
Fixed - Dependencies of JavaScript assets

Credits

Props @cadic, @psorensen, @adamsilverstein, @dkotter

peterwilsoncc

While testing, I notice the insecure element only has the border added if the "scroll to element" button is clicked. Is this intentional?

(Gif only shows block editor but the same happens in the classic editor)

🔢 Is used to indicate the comment applies elsewhere in the PR.

peterwilsoncc · 2022-05-25T23:11:18Z

src/js/gutenberg.js

@@ -1,6 +1,10 @@
 import { debounce } from 'underscore';


There are a few items in use in this file that are not listed in the dependencies (jQuery, apiRequest, domReady). Some of these are legacy issues but it would be good to add them to ensure this update doesn't end up changing the order in which files are included.

insecure-content-warning/includes/assets.php

Lines 34 to 40 in 42a8fbf

wp_enqueue_script(

'insecure-content-gutenberg',

INSECURE_CONTENT_URL . 'dist/js/gutenberg.js',

array( 'wp-components', 'wp-data', 'wp-dom', 'wp-editor', 'wp-element', 'wp-edit-post', 'wp-i18n', 'wp-plugins' ),

INSECURE_CONTENT_VERSION,

true

);

🔢 To avoid a duplicate comment, are you able to review both JS files dependencies?

insecure-content-warning/includes/assets.php

Lines 60 to 66 in 42a8fbf

wp_enqueue_script(

'insecure-content-admin',

INSECURE_CONTENT_URL . 'dist/js/classic-editor.js',

array( 'wp-i18n' ),

INSECURE_CONTENT_VERSION,

true

);

includes/assets.php

cadic · 2022-05-26T10:11:02Z

While testing, I notice the insecure element only has the border added if the "scroll to element" button is clicked. Is this intentional?

From the original issue report and discussion, the highlight expected to happen on an explicit action. Thanks to your demo, it looks like we should avoid highlighting multiple items at a time, I will add "blur all" action before highlighting new element.

Co-authored-by: Peter Wilson <[email protected]>

cadic · 2022-05-26T10:18:20Z

Added a note about multiple elements with the same URL to PR description

peterwilsoncc · 2022-05-27T01:33:19Z

The changes are working as expected, only one image is highlighted at a time.

I did notice in the block editor that the code will no longer scroll to an element if the post is unsaved. Again a gif to demonstrate:

The steps I am taking are:

Create a post
Add some content, insecure images
Click publish, warning show
Click view element -- scrolling fails
Click save draft
Click view element -- scrolling fails
Go to post list view
Reopen post
Click publish
Click view element -- scrolling succeeds

I am on leave next week so you'll need to reassign this issue for review.

This reverts commit c6cedbe.

peterwilsoncc

This is now working as expected, are you able to take a look at this earlier comment about dependencies to ensure all required files are listed for the JS files?

https://github.com/10up/insecure-content-warning/pull/73/files#r882190332

This reverts commit b7c1597.

cadic · 2022-12-16T07:07:06Z

@peterwilsoncc I've added Webpack Dependency Extractor Plugin to handle with dependencies

peterwilsoncc

Sorry Max, one final thing due to the dependency extractor not including underscore.

config/webpack.config.common.js

Co-authored-by: Peter Wilson <[email protected]>

peterwilsoncc

Thanks Max, I checked the built assets and they now include underscore.

The eslint and dependency failures look to be coming from elsewhere so I don't think they need to block merge.

jeffpaul · 2023-01-09T18:45:37Z

@dkotter will leave it up to you if you want to merge this in ahead of the 1.0.3 release or not

peterwilsoncc

Still happy with this.

For some reason the eslint and cypress jobs have started failing since I last looked at this repo. I did a quick test in another branch and it seems unrelated to this PR.

cadic added 4 commits May 24, 2022 17:40

add node packages for scroll

e9172a1

style for view button

4158bd0

Add view button, block or element scroll into view

ffeb551

Dist

9923f0a

cadic self-assigned this May 24, 2022

jeffpaul added this to the 1.1.0 milestone May 24, 2022

cadic added 6 commits May 25, 2022 11:44

Add styles for insecure element

bde38f6

Highlight insecure element, blur the highlight before save

437c26c

dist

62b897c

blur before every check

f4a2c90

highlight and blur in block editor

29ad1b8

dist

e6dfb3f

cadic changed the title ~~Feature/highlight~~ Highlight insecure element and scroll to it May 25, 2022

cadic requested review from a team, peterwilsoncc and jeffpaul and removed request for a team May 25, 2022 09:40

cadic marked this pull request as ready for review May 25, 2022 09:40

Merge branch 'develop' into feature/highlight

42a8fbf

peterwilsoncc reviewed May 25, 2022

View reviewed changes

cadic marked this pull request as draft May 26, 2022 10:12

Return single $url if css list is empty

d76982a

Co-authored-by: Peter Wilson <[email protected]>

cadic added 2 commits May 26, 2022 13:20

Blur insecure before viewing new element

ace9cd4

dist

b3fa232

jeffpaul requested a review from peterwilsoncc May 26, 2022 18:40

jeffpaul removed request for jeffpaul and peterwilsoncc June 2, 2022 03:18

cadic requested a review from peterwilsoncc November 8, 2022 11:39

Revert "wpcs action version"

7f9305f

This reverts commit c6cedbe.

cadic marked this pull request as ready for review November 8, 2022 14:41

peterwilsoncc reviewed Nov 9, 2022

View reviewed changes

jeffpaul and others added 11 commits November 16, 2022 13:53

Merge branch 'develop' into feature/highlight

a0b6197

Merge branch 'develop' into feature/highlight

2e00989

Use dependency extractor plugin

60c5fc5

Update JS dependencies

a287491

Remove compiled assets and POT files from the repository

b7c1597

Revert "Remove compiled assets and POT files from the repository"

46882b1

This reverts commit b7c1597.

Remove compiled assets from the repo

05e947a

Use dependencies and version from webpack

bb87d16

Remove jquery from externals to be handled by dependency plugin

36d319c

Fix css dependencies/version

9257a2b

Change lint target directory

74055ca

cadic requested a review from peterwilsoncc December 16, 2022 07:07

peterwilsoncc requested changes Dec 19, 2022

View reviewed changes

config/webpack.config.common.js Outdated Show resolved Hide resolved

cadic and others added 2 commits January 6, 2023 20:44

Update config/webpack.config.common.js

4dcbb70

Co-authored-by: Peter Wilson <[email protected]>

Merge branch 'develop' into feature/highlight

e49640f

cadic requested a review from peterwilsoncc January 6, 2023 17:52

peterwilsoncc previously approved these changes Jan 8, 2023

View reviewed changes

Merge branch 'develop' into feature/highlight

197e49a

cadic dismissed peterwilsoncc’s stale review via 197e49a February 19, 2023 19:57

cadic requested a review from a team as a code owner February 19, 2023 19:57

jeffpaul requested a review from peterwilsoncc February 28, 2023 17:45

peterwilsoncc approved these changes Mar 1, 2023

View reviewed changes

jeffpaul merged commit 85ccd6b into develop Mar 1, 2023

jeffpaul deleted the feature/highlight branch March 1, 2023 22:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Highlight insecure element and scroll to it #73

Highlight insecure element and scroll to it #73

cadic commented May 24, 2022 •

edited

Loading

peterwilsoncc left a comment

peterwilsoncc May 25, 2022

cadic commented May 26, 2022

cadic commented May 26, 2022

peterwilsoncc commented May 27, 2022

peterwilsoncc left a comment

cadic commented Dec 16, 2022

peterwilsoncc left a comment

peterwilsoncc left a comment

jeffpaul commented Jan 9, 2023

peterwilsoncc left a comment

	wp_enqueue_script(
	'insecure-content-gutenberg',
	INSECURE_CONTENT_URL . 'dist/js/gutenberg.js',
	array( 'wp-components', 'wp-data', 'wp-dom', 'wp-editor', 'wp-element', 'wp-edit-post', 'wp-i18n', 'wp-plugins' ),
	INSECURE_CONTENT_VERSION,
	true
	);

	wp_enqueue_script(
	'insecure-content-admin',
	INSECURE_CONTENT_URL . 'dist/js/classic-editor.js',
	array( 'wp-i18n' ),
	INSECURE_CONTENT_VERSION,
	true
	);

Highlight insecure element and scroll to it #73

Highlight insecure element and scroll to it #73

Conversation

cadic commented May 24, 2022 • edited Loading

Description of the Change

Verification Process

Possible Drawbacks

Checklist:

Changelog Entry

Credits

peterwilsoncc left a comment

Choose a reason for hiding this comment

peterwilsoncc May 25, 2022

Choose a reason for hiding this comment

cadic commented May 26, 2022

cadic commented May 26, 2022

peterwilsoncc commented May 27, 2022

peterwilsoncc left a comment

Choose a reason for hiding this comment

cadic commented Dec 16, 2022

peterwilsoncc left a comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

jeffpaul commented Jan 9, 2023

peterwilsoncc left a comment

Choose a reason for hiding this comment

cadic commented May 24, 2022 •

edited

Loading