Problem/Motivation

Please check this issue https://github.com/GoogleChrome/lighthouse/issues/11969
As said - document.write() is a winner at this place https://git.drupalcode.org/project/seckit/-/blob/8.x-1.x/js/seckit.docum... , but actually yep, it's better to redo this featured thing without js somehow, because seckit.no_body.css is anyway loading always, but not used, when commented. Means.. why need to load it then?

css

head

CommentFileSizeAuthor
#29 seckit-3193443-29.patch3.18 KBmanish.upadhyay
#28 seckit-3193443-27.patch2.76 KBmanish.upadhyay
#26 seckit-3193443-21.patch1.36 KBmanish.upadhyay
#21 seckit-3193443-20.patch2.89 KBalina.basarabeanu
#17 seckit-3193443-17.patch2.83 KBjohn.oltman
#13 seckit-3193443-13.patch1.85 KBjohn.oltman
#11 seckit-3193443-11.patch1.39 KBjohn.oltman
#6 3193443-6.patch626 bytesPooja Ganjage
#3 3193443-3.patch361 bytesPooja Ganjage
Снимок экрана 2021-01-19 в 10.47.25.png112.63 KBkostyashupenko
Снимок экрана 2021-01-19 в 10.49.32.png50.54 KBkostyashupenko

Issue fork seckit-3193443

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kostyashupenko created an issue. See original summary.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Issue summary: View changes
Pooja Ganjage’s picture

Pooja Ganjage commented
StatusFileSize
new 3193443-3.patch361 bytes

Hi,

Creating a patch for this issue.

Please review the patch.

Let me know if any suggestions.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage commented
Status: Active » Needs review
kumar ashutosh’s picture

kumar ashutosh commented

Patch #3 used, but the screen returns nothing. CSS from the Seckit module apply body { display:none; } . I think that is not good option.

Pooja Ganjage’s picture

Pooja Ganjage commented
StatusFileSize
new 3193443-6.patch626 bytes

Hi,

Creating an updated patch.

Please review.

Let me know if any suggestions.

Thanks.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented

@pooja-ganjage, thanks for working on this issue, but your patches tells that it looks like you don't understand what you are doing at all

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented

Described issue is about one css file, which shouldn't be loaded if no need.
Currently seckit.no_body.css loads always, but hidden if some js conditions. Js file should be removed and logic of load/not load seckit.no_body.css file should be done from php

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
raghwendra’s picture

raghwendra commented

I am also facing problme with "Avoid using document.write('

john.oltman’s picture

john.oltman commented
StatusFileSize
new seckit-3193443-11.patch1.39 KB

Do not use

john.oltman’s picture

john.oltman commented
john.oltman’s picture

john.oltman commented
StatusFileSize
new seckit-3193443-13.patch1.85 KB

Do not use

The last submitted patch, 11: seckit-3193443-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 13: seckit-3193443-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

john.oltman’s picture

john.oltman commented
john.oltman’s picture

john.oltman commented
StatusFileSize
new seckit-3193443-17.patch2.83 KB

Revised patch applies to 8.x and 2.x branches. Instead of doing a document.write, it sets the display property on the body field.

alina.basarabeanu’s picture

alina.basarabeanu commented

Patch #17 works on Drupal core 9.4.8, PHP 8.0.21 and Seckit 2.0.0 even if the tests are failing.
Is there any chance to get merged into dev or a new stable release?

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Assigned: Unassigned » rajeshreeputra
jjsanz’s picture

jjsanz
Primary language Spanish
commented

The latest patch breacks "better toolbar" padding top...

alina.basarabeanu’s picture

alina.basarabeanu commented
StatusFileSize
new seckit-3193443-20.patch2.89 KB

Just discovered that the latest patch breaks the XML sitemap.
When viewing the website sitemap in Firefox an error is shown: XML Parsing Error: not well-formed
Set the defer attribute as defer="defer" instead of just 'defer' for the seckit.frame_check.js file.
Updated patch added.

alina.basarabeanu’s picture

alina.basarabeanu commented
Status: Needs work » Needs review
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Assigned: rajeshreeputra » Unassigned
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Should this be moved to the 2.0.x branch?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 8.x-1.2 » 2.x-dev

Moving to the 2.0.x/2.x branch because the 8.x-1.x branch is no longer supported.

manish.upadhyay’s picture

manish.upadhyay commented
StatusFileSize
new seckit-3193443-21.patch1.36 KB

Status: Needs review » Needs work

The last submitted patch, 26: seckit-3193443-21.patch, failed testing. View results

manish.upadhyay’s picture

manish.upadhyay commented
StatusFileSize
new seckit-3193443-27.patch2.76 KB

Updated patch.

manish.upadhyay’s picture

manish.upadhyay commented
StatusFileSize
new seckit-3193443-29.patch3.18 KB

chaitanyadessai made their first commit to this issue’s fork.

chaitanyadessai opened merge request !28

chaitanyadessai closed merge request !28

chaitanyadessai opened merge request !29

chaitanyadessai closed merge request !29

luismagr made their first commit to this issue’s fork.

luismagr opened merge request !30

luismagr’s picture

luismagr commented

Hi,

I've experienced the same issue while trying to run storybook on a drupal installation with seckit installed on it. I've created a MR that prevent the error. Probably there's a plan to fix this in another way but for now, it seems this works well for my issue.

Happy to continue with the issue if someone points me to a better direction.

Thanks

the_g_bomb made their first commit to this issue’s fork.

the_g_bomb’s picture

the_g_bomb commented

Hi,
@luismagr there is a solution offered already that solves this in #21, your MR in #30 does not include the complete solution.
@manish.upadhyay I think your patch at #29 is close, but I don't think defer="defer" can be used for css

the_g_bomb opened merge request !46

the_g_bomb’s picture

the_g_bomb commented
Status: Needs work » Needs review

Pushed a reroll of #21 applying to the 2.x branch.
This patch can be used if needed:
https://git.drupalcode.org/project/seckit/-/merge_requests/46.diff

sergiur’s picture

sergiur
he/him
Primary language English
Location London, UK
commented
Status: Needs review » Reviewed & tested by the community

I tested this patch and it seems to be working correctly.

To test I disabled clickjacking protection (seckit_clickjacking.x_frame = 0) on one site and iframed it in another site. The other site showed a white/empty iframe, with the following message in console:

Uncaught SecurityError: Failed to read a named property 'hostname' from 'Location': Blocked a frame with origin "https://[site]" from accessing a cross-origin frame.

This patch also has the added benefit of fixing the XML Sitemap page when stylesheets are enabled, so it should solve this issue at the same time: #3219252: Breaks sitemap.xml when JS +CSS + Noscript protection is enabled

Thank you!