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

Add a scriptlet to honor noscript tag #2437

Closed
8 tasks done
Yuki2718 opened this issue Jan 1, 2023 · 23 comments
Closed
8 tasks done

Add a scriptlet to honor noscript tag #2437

Yuki2718 opened this issue Jan 1, 2023 · 23 comments
Labels
duplicate This issue or pull request already exists

Comments

@Yuki2718
Copy link

Yuki2718 commented Jan 1, 2023

Prerequisites

  • I verified that this is not a filter list issue. Report any issues with filter lists or broken website functionality in the uAssets issue tracker.
  • This is not a support issue or a question. For support, questions, or help, visit /r/uBlockOrigin.
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue.
  • The issue is not present after disabling uBO in the browser.
  • I checked the documentation to understand that the issue I am reporting is not normal behavior.

I tried to reproduce the issue when...

  • uBO is the only extension.
  • uBO uses default lists and settings.
  • using a new, unmodified browser profile.

Description

AdguardTeam/Scriptlets#274
I emphasize currently there's no fix for Firefox with ETP enabled. If the scriptlet is made available, we can fix the issues without current exception in uBlock filters and Japanese filter (see AdguardTeam/AdguardFilters#137213 (comment)). I guess it's not too much work as uBO already has noscript-spoof.js

A specific URL where the issue occurs.

https://soranews24.com/

Steps to Reproduce

  1. Visit the site with Firefox ETP enabled
  2. See images are not loaded.
  3. If you turn No-scripting switch on, images will be loaded because now noscript is honored

Expected behavior

Filter maintainer should have a way to honor noscript for such a case

Actual behavior

No way

uBO version

1.46.1b3

Browser name and version

Firefox 108.0.1

Operating System and version

Windows 10

@uBlock-user uBlock-user added the enhancement New feature or request label Jan 1, 2023
@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

Images are loaded on my side with ETP enabled (default).

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 2, 2023

@gorhill Really? Test on Private tab and scroll down a bit.

soranews

@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

On Private tab, the first top two images are loaded, but the others are placeholders with text "Please allow ads on our site...".


Disabling ETP solves the issue, so shouldn't this be reported to Firefox devs?

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 2, 2023

isabling ETP solves the issue, so shouldn't this be reported to Firefox devs?

Do they address anti-adb? The blocking is legit. I don't know if ETP support per-page exception.

@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

It's kind of the realm of using other blockers beside uBO can cause such issues, in which case disabling the other blocker(s) is the right solution, rather than trying to fix in uBO something caused by another blocker.

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 2, 2023

Except that this is a browser's built-in feature which we had not been recommending to turn it off.

@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

It also happens without uBO.

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 2, 2023

Yes, because currently exception is mandatory to fix. If noscript tag can be honored, no exception will be needed.

@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

Note that ETP icon has a "Site not working?" link from which we can send a report. To me this seems the more sensible option, rather than burdening uBO with handling an issue caused by an external blocker.

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 2, 2023

Okay sent a report. But I wonder, can't it be mostly copy-and-paste of noscript-spoof.js except for an argument I suggested? Sure, I don't know the fact.

@gorhill
Copy link
Member

gorhill commented Jan 2, 2023

What are the chance such scriptlet would be useful elsewhere than on those two related sites? (rocket and sora news24)

@troysjanda
Copy link

troysjanda commented Jan 2, 2023

Retracted due to not following the steps

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 3, 2023

What are the chance such scriptlet would be useful elsewhere than on those two related sites?

I have to admit I'm not aware of. Though it will be another issue with different fix, as uMatrix is now archived, gorhill/uMatrix#319 is to some extent relevant - but probably better to add noscript-spoof option/switch to uBO than filter scriptlet in terms of usability.

@Yuki2718
Copy link
Author

Yuki2718 commented Jan 3, 2023

I don't have ETP enable and it does not load the images either except the 1st two. using FF 108, Ubo 1.46.1b4, windows 10

Unable to reproduce, and such issues should first be reported to uBlockOrigin/uAssets#15705 with all the required details.

@troysjanda
Copy link

troysjanda commented Jan 3, 2023

Unable to reproduce, and such issues should first be reported to uBlockOrigin/uAssets#15705 with all the required details.

1st off that issue is closed however I updated this and #15705 as requested.

@krystian3w
Copy link

This look as trust level needed #2229.

@uBlock-user
Copy link
Contributor

uBlock-user commented Jan 3, 2023

How about a renaming-attribute scriptlet ?

/// rename-attr.js
(() => {
                'use strict';
                const selector = '{{1}}';
		if ( selector === '' || selector === '{{1}}' ) { return; }
		const oldattr = '{{2}}';
	        const newattr = '{{3}}';
		const renameattr = ev => {
	        				if ( ev ) { self.removeEventListener(ev.type, renameattr, true); }
						const elems = document.querySelectorAll( selector );
						try {
							for ( const elem of elems ) {
								if ( elem.hasAttribute( oldattr ) ) {
							            const value = elem.getAttribute( oldattr );
		                                                    elem.removeAttribute( oldattr );
		     				   	   	    elem.setAttribute( newattr, value );
								}
							}	
						} catch { }
		};
		if (document.readyState === 'loading') {
		    	    self.addEventListener('DOMContentLoaded', renameattr, true);
	   	} else {
		    	    renameattr();
	   	}
})();

With the above scriptlet, you need the following filters --

soranews24.com##+js(rename-attr, img[data-sco-src], data-sco-src, src)
soranews24.com##.aligncenter.lazy[data-sco-srcset]:style(opacity:1 !important)

For rocketnews24 Add -

rocketnews24.com##+js(ra, id, #div-gpt-ad-footer\, #div-gpt-ad-pagebottom\, #div-gpt-ad-relatedbottom-1\, #div-gpt-ad-sidebottom)
@@||rocketnews24.com^$ghide
*$script,redirect-rule=noopjs,domain=rocketnews24.com

and then

rocketnews24.com##+js(rename-attr, img[data-sco-src], data-sco-src, src)
rocketnews24.com##.aligncenter.lazy[data-sco-srcset]:style(opacity:1 !important)

@MasterKia
Copy link
Member

#2347 ?

@gwarser
Copy link

gwarser commented Jan 4, 2023

How about a renaming-attribute scriptlet ?

"Override src" hardcoded to img tags, src as a hardcoded target and one of this element attributes as a source value for src?

(few more sources: source, source, source 😄 )

@uBlock-user
Copy link
Contributor

uBlock-user commented Jan 4, 2023

@gwarser ?

(few more sources: source, source, source 😄 )

There can only be one "src" attribute present in any inline-tag, so that will never be a problem.

@gwarser
Copy link

gwarser commented Jan 4, 2023

This was only a joke, because of many "source" words in one sentence :)

@uBlock-user
Copy link
Contributor

Right, I only understood after I left the comment, anyway your comment was helpful as it reminded me that I hadn't simulated the "renaming" appearance as I intended, and that was fixed.

@Yuki2718
Copy link
Author

Yuki2718 commented Feb 2, 2023

Closing for #2347 which has better general utility than this suggestion.

@Yuki2718 Yuki2718 closed this as completed Feb 2, 2023
@uBlock-user uBlock-user added duplicate This issue or pull request already exists and removed enhancement New feature or request labels Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

7 participants