Add 'Copy with Redirect' context menu feature#443
Add 'Copy with Redirect' context menu feature#443SmartManoj wants to merge 2 commits intoeinaregilsson:masterfrom
Conversation
Introduces a context menu option to copy redirected URLs directly from links, applying user-defined redirect rules. Updates background script to handle context menu creation, click events, and clipboard operations, and updates permissions in manifest.json. Documentation in README.md is expanded to describe the new feature.
Gitoffthelawn
left a comment
There was a problem hiding this comment.
Looks nice. Thank you. ❤️
I have mixed emotions about a menuitem that will copy the original link when there is no applicable redirect rule, but copies a modified URL when there is one.
Here are 3 other options. I'm interested in your thoughts about these 3 alternative UX designs (they are all mutually exclusive; only the original UX design from the PR or one of these will be used):
- Dim the new context menuitem when there is no applicable redirect rule.
- Do not show the new context menuitem when there is no applicable redirect rule.
- Change the menuitem's label to reflect whether or not there is an applicable redirect rule.
|
Will this item pollute the context menu? |
Somewhat. That's one reason I like option 2:
|
Added logic to update the context menu dynamically depending on whether there are matching redirect rules for a given URL. The context menu is now only shown when relevant, and is removed when the extension is disabled. This improves user experience by preventing unnecessary context menu items.
Nanba, also allow users to disable the context menu completely. |
There was a problem hiding this comment.
Thoughts on first glance, I'll do another review:
- Currently,
Copy with Redirectalways shows. - Currently, pressing
Copy with Redirect, when the link has no redirect, results in a noop (nothing being copied to your clipboard). - You're using
varunnecessarily overconsts andlets.
| function hasMatchingRedirects(url) { | ||
| if (!partitionedRedirects || Object.keys(partitionedRedirects).length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if there are any enabled redirect rules that match this URL | ||
| for (var requestType in partitionedRedirects) { | ||
| var list = partitionedRedirects[requestType]; | ||
| if (list) { | ||
| for (var i = 0; i < list.length; i++) { | ||
| var r = list[i]; | ||
| if (!r.disabled) { | ||
| var result = r.getMatch(url); | ||
| if (result.isMatch) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Setup context menu for "Copy with Redirect" feature | ||
| function setupContextMenu() { | ||
| chrome.contextMenus.removeAll(function() { | ||
| chrome.contextMenus.create({ | ||
| id: "copyWithRedirect", | ||
| title: "Copy with Redirect", | ||
| contexts: ["link"], | ||
| documentUrlPatterns: ["http://*/*", "https://*/*"] | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| // Handle context menu clicks | ||
| chrome.contextMenus.onClicked.addListener(function(info, tab) { | ||
| if (info.menuItemId === "copyWithRedirect") { | ||
| // Only proceed if there are matching redirects for this URL | ||
| if (hasMatchingRedirects(info.linkUrl)) { | ||
| handleCopyWithRedirect(info.linkUrl, tab); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Handle the "Copy with Redirect" functionality | ||
| function handleCopyWithRedirect(originalUrl, tab) { | ||
| // Check if there are any redirects that would apply to this URL | ||
| var hasRedirect = false; | ||
| var redirectedUrl = originalUrl; | ||
|
|
||
| // Check all redirect types for a match | ||
| for (var requestType in partitionedRedirects) { | ||
| var list = partitionedRedirects[requestType]; | ||
| if (list) { | ||
| for (var i = 0; i < list.length; i++) { | ||
| var r = list[i]; | ||
| var result = r.getMatch(originalUrl); | ||
|
|
||
| if (result.isMatch) { | ||
| hasRedirect = true; | ||
| redirectedUrl = result.redirectTo; | ||
| break; | ||
| } | ||
| } | ||
| if (hasRedirect) break; | ||
| } | ||
| } | ||
|
|
||
| // Copy the appropriate URL to clipboard | ||
| var urlToCopy = hasRedirect ? redirectedUrl : originalUrl; | ||
|
|
||
| // Use the clipboard API to copy the URL | ||
| navigator.clipboard.writeText(urlToCopy).then(function() { | ||
| // Show a notification that the URL was copied | ||
| if (enableNotifications) { | ||
| let icon = isDarkMode() ? "images/icon-dark-theme-48.png": "images/icon-light-theme-48.png"; | ||
|
|
||
| if(navigator.userAgent.toLowerCase().indexOf("chrome") > -1 && navigator.userAgent.toLowerCase().indexOf("opr")<0){ | ||
| var items = [{title:"Original URL: ", message: originalUrl},{title:"Copied URL: ",message: urlToCopy}]; | ||
| var head = "Redirector - Copied URL"; | ||
| chrome.notifications.create({ | ||
| type : "list", | ||
| items : items, | ||
| title : head, | ||
| message : head, | ||
| iconUrl : icon | ||
| }); | ||
| } else { | ||
| var message = hasRedirect ? | ||
| "Copied redirected URL: " + urlToCopy + " (original: " + originalUrl + ")" : | ||
| "Copied URL: " + urlToCopy; | ||
|
|
||
| chrome.notifications.create({ | ||
| type : "basic", | ||
| title : "Redirector", | ||
| message : message, | ||
| iconUrl : icon | ||
| }); | ||
| } | ||
| } | ||
| }).catch(function(err) { | ||
| log('Failed to copy URL to clipboard: ' + err); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
| function hasMatchingRedirects(url) { | |
| if (!partitionedRedirects || Object.keys(partitionedRedirects).length === 0) { | |
| return false; | |
| } | |
| // Check if there are any enabled redirect rules that match this URL | |
| for (var requestType in partitionedRedirects) { | |
| var list = partitionedRedirects[requestType]; | |
| if (list) { | |
| for (var i = 0; i < list.length; i++) { | |
| var r = list[i]; | |
| if (!r.disabled) { | |
| var result = r.getMatch(url); | |
| if (result.isMatch) { | |
| return true; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| return false; | |
| } | |
| // Setup context menu for "Copy with Redirect" feature | |
| function setupContextMenu() { | |
| chrome.contextMenus.removeAll(function() { | |
| chrome.contextMenus.create({ | |
| id: "copyWithRedirect", | |
| title: "Copy with Redirect", | |
| contexts: ["link"], | |
| documentUrlPatterns: ["http://*/*", "https://*/*"] | |
| }); | |
| }); | |
| } | |
| // Handle context menu clicks | |
| chrome.contextMenus.onClicked.addListener(function(info, tab) { | |
| if (info.menuItemId === "copyWithRedirect") { | |
| // Only proceed if there are matching redirects for this URL | |
| if (hasMatchingRedirects(info.linkUrl)) { | |
| handleCopyWithRedirect(info.linkUrl, tab); | |
| } | |
| } | |
| }); | |
| // Handle the "Copy with Redirect" functionality | |
| function handleCopyWithRedirect(originalUrl, tab) { | |
| // Check if there are any redirects that would apply to this URL | |
| var hasRedirect = false; | |
| var redirectedUrl = originalUrl; | |
| // Check all redirect types for a match | |
| for (var requestType in partitionedRedirects) { | |
| var list = partitionedRedirects[requestType]; | |
| if (list) { | |
| for (var i = 0; i < list.length; i++) { | |
| var r = list[i]; | |
| var result = r.getMatch(originalUrl); | |
| if (result.isMatch) { | |
| hasRedirect = true; | |
| redirectedUrl = result.redirectTo; | |
| break; | |
| } | |
| } | |
| if (hasRedirect) break; | |
| } | |
| } | |
| // Copy the appropriate URL to clipboard | |
| var urlToCopy = hasRedirect ? redirectedUrl : originalUrl; | |
| // Use the clipboard API to copy the URL | |
| navigator.clipboard.writeText(urlToCopy).then(function() { | |
| // Show a notification that the URL was copied | |
| if (enableNotifications) { | |
| let icon = isDarkMode() ? "images/icon-dark-theme-48.png": "images/icon-light-theme-48.png"; | |
| if(navigator.userAgent.toLowerCase().indexOf("chrome") > -1 && navigator.userAgent.toLowerCase().indexOf("opr")<0){ | |
| var items = [{title:"Original URL: ", message: originalUrl},{title:"Copied URL: ",message: urlToCopy}]; | |
| var head = "Redirector - Copied URL"; | |
| chrome.notifications.create({ | |
| type : "list", | |
| items : items, | |
| title : head, | |
| message : head, | |
| iconUrl : icon | |
| }); | |
| } else { | |
| var message = hasRedirect ? | |
| "Copied redirected URL: " + urlToCopy + " (original: " + originalUrl + ")" : | |
| "Copied URL: " + urlToCopy; | |
| chrome.notifications.create({ | |
| type : "basic", | |
| title : "Redirector", | |
| message : message, | |
| iconUrl : icon | |
| }); | |
| } | |
| } | |
| }).catch(function(err) { | |
| log('Failed to copy URL to clipboard: ' + err); | |
| }); | |
| } | |
| function setupContextMenu() { | |
| chrome.contextMenus.removeAll(function() { | |
| chrome.contextMenus.create({ | |
| id: "copyWithRedirect", | |
| title: "Copy with Redirect", | |
| contexts: ["link"], | |
| documentUrlPatterns: ["http://*/*", "https://*/*"] | |
| }); | |
| }); | |
| } | |
| // Handle the "Copy with Redirect" functionality | |
| const handleCopyWithRedirect = async url => { | |
| let hasRedirect = false; | |
| // Check all redirect types for a match | |
| partitionedLoop: | |
| for (const requestType in partitionedRedirects) { | |
| for (let r of partitionedRedirects[requestType]) { | |
| if (!r.disabled && (r = r.getMatch(url)).isMatch) { | |
| hasRedirect = true; | |
| // Copy the appropriate URL to clipboard | |
| url = r.redirectTo; | |
| break partitionedLoop; | |
| } | |
| } | |
| } | |
| // Use the clipboard API to copy the URL | |
| try { | |
| await navigator.clipboard.writeText(url) | |
| // Show a notification that the URL was copied | |
| if (enableNotifications) { | |
| const iconUrl = `images/icon-${isDarkMode() ? "dark" : "light"}-theme-48.png`; | |
| if (navigator.userAgent.includes(" Chrome/")) { | |
| const | |
| items = [ | |
| { title: "Original URL: ", message: url }, | |
| { title: "Copied URL: ", message: url } | |
| ], | |
| head = "Redirector - Copied URL"; | |
| chrome.notifications.create({ | |
| type : "list", | |
| items, | |
| title : head, | |
| message : head, | |
| iconUrl | |
| }); | |
| } else { | |
| const message = hasRedirect | |
| ? "Copied redirected URL: " + url | |
| : "Copied URL: " + url; | |
| chrome.notifications.create({ | |
| type : "basic", | |
| title : "Redirector", | |
| message, | |
| iconUrl | |
| }); | |
| } | |
| } | |
| } | |
| catch (err) { | |
| log('Failed to copy URL to clipboard: ' + err.message); | |
| }; | |
| } | |
| // Handle context menu clicks | |
| chrome.contextMenus.onClicked.addListener(info => { | |
| // Only proceed if there are matching redirects for this URL | |
| if (info.menuItemId === "copyWithRedirect") { | |
| handleCopyWithRedirect(info.linkUrl); | |
| } | |
| }); | |
- Remove duplicated logic.
- Reduce unnecessary branches.
- Use async/await.
- Use loop labels.
- Use the
Array.prototype.valuesbuilt in iterator. - Remove spaces on empty lines.
- Remove unused function params.
- Use
const/letinstead ofvar. - Name variables the same as object properties (DRY).
- Prefer chronological arrow functions.
- Use the correct
DOMExceptionproperty. - Your code loops through
partitionedRedirectsa whole 3 TIMES, you only need to once.Object.keys(completely unnecessary),hasMatchingRedirects(completely duplicated logic btw), andhandleCopyWithRedirect. Not only this, but you didn't break the loop properly.
There was a problem hiding this comment.
Now it will show for all links. The function should be
getMatchingRedirects.
That's just not true, you don't understand your own code, there's a link context.
Original URL and Copied URL belongs to same now.
I chose to remove the Original URL, because there was no point in keeping that. It's already in the bottom left corner when hovering.
|
Closing in favor of #448 |


Introduces a context menu option to copy redirected URLs directly from links, applying user-defined redirect rules. Updates background script to handle context menu creation, click events, and clipboard operations, and updates permissions in manifest.json. Documentation in README.md is expanded to describe the new feature.
Resolves #414