Skip to content

update DNR samples to work in new DCC#1083

Open
patrickkettner wants to merge 2 commits into
mainfrom
update-dnr-sample
Open

update DNR samples to work in new DCC#1083
patrickkettner wants to merge 2 commits into
mainfrom
update-dnr-sample

Conversation

@patrickkettner

Copy link
Copy Markdown
Collaborator

fixes #1082

@oliverdunk oliverdunk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@hrmtsh2

hrmtsh2 commented Mar 18, 2024

Copy link
Copy Markdown

Hi, I came here from the discussion on #1082 as I was having the same issue. However this fix also doesn't seem to change things, and somewhere down the line the changes were reverted?
I was trying to run this sample (in relation to my potential work on building an interactive tool to view changes in rules for DNR-bassed extensions for the GSoC program), but it doesn't seem to work.
As mentioned in #1082 by @oliverdunk, I tried to keep both "main_frame", "xmlhttprequest" in the resourceTypes key to accommodate both sites with service workers and with main_frame, but to little avail. The only change I see is that the console message in service_worker.js now shows the message confirming the redirection (I assume the tabId of -1 indicates that it is not tab-specific?).
However, the redirection still doesn't occur. Kindly tell me where I am going wrong.

Edit: I understand that the original user with the issue also didn't see anything get fixed when they tried this fix. However, they did see the desired outcome when they removed the resourceType section altogether. This doesn't seem to happen on my end, however. There is a message, as before, indicating the redirection, but no actual redirection occurs.

developer.chrome.com serves navigations through its service worker for
returning visitors, so there is no main_frame network request for these
rules to match. Match the service worker's xmlhttprequest as well, and
update the redirect destinations that have since moved.
@patrickkettner

Copy link
Copy Markdown
Collaborator Author

@oliverdunk finally got back to this. I dug into why the reports here were inconsistent: developer.chrome.com serves navigations through its service worker for returning visitors, so there is no main_frame network request for DNR to match. Fresh profiles redirect fine, returning visitors get nothing, which lines up with what @hrmtsh2 saw (rule match logged with tabId -1, no actual redirect).

Matching xmlhttprequest as well (your suggestion from #1082) fixes it on current Chrome. I tested this branch on 149 with a fresh profile and again with the site's service worker registered, and both redirect now. Also updated the redirect destinations since the old ones have themselves become 301s, and added a README note about the service worker behavior.

Given the sample now needs that caveat explained to make sense, does it still seem worth merging, or would you rather we abandon it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manifest V3 declarativeNetRequest redirect sample is completely broken on latest chrome 121

3 participants