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

remove-attr does not always work #1445

Closed
dimisa-RUAdList opened this issue Jan 7, 2021 · 23 comments
Closed

remove-attr does not always work #1445

dimisa-RUAdList opened this issue Jan 7, 2021 · 23 comments
Labels
fixed issue has been addressed

Comments

@dimisa-RUAdList
Copy link

Example: https://news.rambler.ru/conflicts/45563172-smi-politsiya-ottesnila-protestuyuschih-so-stupeney-kapitoliya/

When scrolling down the strings, the rule rambler.ru##+js(remove-attr, class, .j-mini-player__video) should fix the player in a static position, but for some reason this does not happen. In version 1.31.2, this problem is absent and the rule works fine.

Config

Google Chrome 87.0.4280.88
Firefox 84.0.2
uBlock Origin v1.32.5b7 default + RU AdList, Counters

@gwarser
Copy link

gwarser commented Jan 7, 2021

Attribute is removed correctly and then new is created with j-mini-player__video--absolute. Some timing issue?

@uBlock-user uBlock-user added the something to address something to address label Jan 7, 2021
@gorhill
Copy link
Member

gorhill commented Jan 7, 2021

I can't find an element matching .j-mini-player__video at that page. What do I need to do for such element to be present?

@gorhill
Copy link
Member

gorhill commented Jan 7, 2021

Loos to me this work better if the goal is to prevent fixed position:

rambler.ru##.j-mini-player__container:style(position: initial !important)

@gwarser
Copy link

gwarser commented Jan 7, 2021

I can't find an element matching .j-mini-player__video at that page.

Removed by Ruadlist.

@gorhill
Copy link
Member

gorhill commented Jan 7, 2021

Duh sorry, I couldn't find it because remove-attr worked on my side... So the issue is probably because remove-attr does not listen to DOM mutations.

@uBlockOrigin uBlockOrigin locked as spam and limited conversation to collaborators Jan 7, 2021
@uBlock-user
Copy link
Contributor

So the issue is probably because remove-attr does not listen to DOM mutations.

Yes, that's the issue, also it wouldn't work with 1.31.2 either for that matter.

@uBlockOrigin uBlockOrigin unlocked this conversation Jan 7, 2021
@dimisa-RUAdList
Copy link
Author

dimisa-RUAdList commented Jan 7, 2021

The rule rambler.ru##+js(remove-attr, class, .j-mini-player__video) has been working for a long time and stably: Gif

To see the j-mini-player__video element, use rambler.ru#@#+js(remove-attr, class, .j-mini-player__video).

Previously, :style type rules were used instead. But they did not always allow to keep the size of the player window. I can go back to them, but I would like to revive remove-attr.

Add
For some reason, the previous video with version v1.32.5b7 was not fully displayed. There is probably some kind of duration limitation. Here it is separately: Gif

@gorhill
Copy link
Member

gorhill commented Jan 7, 2021

I will add support for mutation observer in remove-attr in next dev build.

@dimisa-RUAdList

This comment has been minimized.

@gwarser

This comment has been minimized.

@dimisa-RUAdList

This comment has been minimized.

@krystian3w

This comment has been minimized.

gorhill added a commit to gorhill/uBlock that referenced this issue Jan 8, 2021
Related issue:
- uBlockOrigin/uBlock-issues#1445

A third (optional) argument has been added to `remove-attr`
scriptlet, which can be one or more space-separated tokens
dictating the behavior of the scriptlet:

`stay`: This tells the scriplet to stay and act on DOM
changes, whiĺe the default behavior is to act only once
when the document becomes interactive.

`complete`: This tells the scriplet to start acting only
when the document is complete, i.e. once all secondary
resources have been loaded, while the default is to start
acting when the document is interactive -- which is earlier
than when the document is complete.

Example:

    ...##+js(remove-attr, class, .j-mini-player, stay)
@gorhill gorhill changed the title remove-attr does not always work in version 1.32.5b7 remove-attr does not always work Jan 8, 2021
@uBlock-user uBlock-user added fixed issue has been addressed and removed something to address something to address labels Jan 8, 2021
@gorhill
Copy link
Member

gorhill commented Jan 8, 2021

@dimisa-RUAdList use a third argument to indicate to the remove-attr scriptlet to stay and act on DOM changes, i.e.:

rambler.ru##+js(remove-attr, class, .j-mini-player__video, stay)

@uBlock-user

This comment has been minimized.

@gorhill

This comment has been minimized.

@dimisa-RUAdList
Copy link
Author

Indeed, in version 1.32.5b8, the old rule rambler.ru##+js(remove-attr, class, .j-mini-player__video) also works fine, without the third argument. However, I find the ability to linger for remove-attr scriplet quite helpful. But I will keep the current rule rambler.ru##+js(addEventListener-defuser, DOMContentLoaded, .j-mini-player__video) for now. In the current stable version 1.32.4 for Firefox, nothing else works except for it (except :style).

JustOff added a commit to gorhill/uBlock-for-firefox-legacy that referenced this issue Jan 25, 2021
Related issue:
- uBlockOrigin/uBlock-issues#1445

A third (optional) argument has been added to `remove-attr`
scriptlet, which can be one or more space-separated tokens
dictating the behavior of the scriptlet:

`stay`: This tells the scriplet to stay and act on DOM
changes, whiĺe the default behavior is to act only once
when the document becomes interactive.

`complete`: This tells the scriplet to start acting only
when the document is complete, i.e. once all secondary
resources have been loaded, while the default is to start
acting when the document is interactive -- which is earlier
than when the document is complete.

Example:

    ...##+js(remove-attr, class, .j-mini-player, stay)

Related commits:
- gorhill/uBlock@0f330c7
- gorhill/uBlock@5fa8739

Co-authored-by: Raymond Hill <rhill@raymondhill.net>
@dimisa-RUAdList
Copy link
Author

I think I found another case.

Latest Google Chrome, Firefox, uBlock Origin 1.34.1rc0 default + RU AdList, Counters

  1. Open: https://www.lostfilm.run/series/DOTA_Dragons_Blood/season_1/episode_6/photos
  2. Click on any photo: https://i.imgur.com/a1a75oy.jpg
  3. Rule lostfilm.run##+js(remove-attr, oncontextmenu)didn't work: https://i.imgur.com/aDaXU9C.jpg

If you refresh the page, then everything works as it should.

@ghost
Copy link

ghost commented Apr 16, 2021

If you refresh the page, then everything works as it should.

  1. Opened gallery by your steps use "Ajax" instead reload page from "0 progress".
  2. Possible change url in address bar without fully reload.
  3. I don't recommend block "Ajax" mechanism to use version scriptlet without , stay.

This may need use , stay:

lostfilm.run##+js(remove-attr, oncontextmenu, stay)

If you must support uBO older than 1.33.0 try:

       userResourcesLocation https://raw.githubusercontent.com/gorhill/uBlock/master/assets/resources/scriptlets.js

uBO for Firefox legacy "preQuantum" (older than 1.16.4.28) maybe needed is build legacy file with older syntax for userResourcesLocation:

# remove attr with dom/node changes
remove-attr.js application/javascript
(function() {
	const token = '{{1}}';
	if ( token === '' || token === '{{1}}' ) { return; }
	const tokens = token.split(/\s*\|\s*/);
	let selector = '{{2}}';
	if ( selector === '' || selector === '{{2}}' ) {
		selector = `[${tokens.join('],[')}]`;
	}
	let behavior = '{{3}}';
	let timer;
	const rmattr = ( ) => {
		timer = undefined;
		try {
			const nodes = document.querySelectorAll(selector);
			for ( let node of nodes ) {
				for ( let attr of tokens ) {
					node.removeAttribute(attr);
				}
			}
		} catch(ex) {
		}
	};
	const mutationHandler = mutations => {
		if ( timer !== undefined ) { return; }
		let skip = true;
		for ( let i = 0; i < mutations.length && skip; i++ ) {
			const { type, addedNodes, removedNodes } = mutations[i];
			if ( type === 'attributes' ) { skip = false; }
			for ( let j = 0; j < addedNodes.length && skip; j++ ) {
				if ( addedNodes[j].nodeType === 1 ) { skip = false; break; }
			}
			for ( let j = 0; j < removedNodes.length && skip; j++ ) {
				if ( removedNodes[j].nodeType === 1 ) { skip = false; break; }
			}
		}
		if ( skip ) { return; }
		if ( 'requestIdleCallback' in self ) {
			timer = self.requestIdleCallback(rmattr, { timeout: 67 });
		} else {
			timer = self.setTimeout(rmattr, 1);
		}
	};
	const start = ( ev ) => {
		if ( ev ) { self.removeEventListener(ev.type, rmattr, true); }
		rmattr();
		if ( /\bstay\b/.test(behavior) === false ) { return; }
		const observer = new MutationObserver(mutationHandler);
		observer.observe(document.documentElement, {
			attributes: true,
			attributeFilter: tokens,
			childList: true,
			subtree: true,
		});
	};
	if ( document.readyState !== 'complete' && /\bcomplete\b/.test(behavior) ) {
		self.addEventListener('load', start, true);
	} else if ( document.readyState === 'loading' ) {
		self.addEventListener('DOMContentLoaded', start, true);
	} else {
		start();
	}
})();

If somebody still base on version older than 1.16.4.28 gorhill/uBlock-for-firefox-legacy@bba2397#diff-e6f727d342440f308271bdcac31b1830a345a5b30d13ae0349be72de85d97751

@D4niloMR
Copy link

D4niloMR commented Feb 14, 2024

2 cases possibly related:

uBlockOrigin/uAssets#21422

and in the images on https://animefire.plus/

animefire.plus##[oncontextmenu], [ondragstart]:remove-attr(/oncontextmenu|ondragstart/) doesn't work, but animefire.plus##+js(ra, oncontextmenu|ondragstart) works.

The issue happens only in Chromium based browsers. I see that with :remove-attr the attribute is gone but the event listener is still there.

Screenshot Left side :remove-attr not used - Right side :remove-attr used

image

@uBlock-user
Copy link
Contributor

uBlock-user commented Feb 14, 2024

@D4niloMR animefire.plus##[oncontextmenu], [ondragstart]:remove-attr(/oncontextmenu|ondragstart/):watch-attr(oncontextment,ondragstart)

try

@D4niloMR
Copy link

@D4niloMR animefire.plus##[oncontextmenu], [ondragstart]:remove-attr(/oncontextmenu|ondragstart/):watch-attr(oncontextment,ondragstart)

Didn't work

@gorhill
Copy link
Member

gorhill commented Feb 14, 2024

The issue happens only in Chromium based browsers

Yes, this is a known issue, discussed elsewhere I can't remember. The procedural cosmetic filters execute in the isolated world, and it seems with Chromium changing on... attribute will not be reflected in the main world's on... property. Hence why the scriptlet remove-attr is still available to take care of such cases for the time being.

@garry-ut99
Copy link

Yes, this is a known issue, discussed elsewhere I can't remember.

Discussed here :

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed issue has been addressed
Projects
None yet
Development

No branches or pull requests

8 participants
@gorhill @gwarser @dimisa-RUAdList @uBlock-user @krystian3w @D4niloMR @garry-ut99 and others