search

pdf / kdeconnect-chrome-extension

A browser extension to send pages and content from your browser to connected KDE Connect devices.
https://chrome.google.com/webstore/detail/kde-connect/ofmplbbfigookafjahpeepbggpofdhbo
MIT License
233 stars 29 forks source link

Investigate separate link contexts #11

Open pdf opened 7 years ago

pdf commented 7 years ago

Mozilla reviewer (Rob W) writes:

here is one thing that you might want to change to improve the functionality. in background.js, every context menu item matches many different contexts, and contextMenuHandler the following logic: info.linkUrl || info.srcUrl || info.frameUrl || info.pageUrl

This may result in surprising behavior, for example if an image is embedded in a link. In this case, it is not possible to share the image, because the extension selects the link.

Consider creating splitting the menu in separate definitions, then up to three menu items can be shown at a given page.

browser.tabs.create({contexts: ["page", "frame"], title: "Share page", ....}) // use info.frameUrl || info.pageUrl in onclick. browser.tabs.create({contexts: ["link"], title: "Share link", ....}) // use info.linkUrl in onclick browser.tabs.create({contexts: ["video"], title: "Share video", ....}) // use info.srcUrl in onclick browser.tabs.create({contexts: ["audio"], title: "Share audio", ....}) // use info.srcUrl in onclick browser.tabs.create({contexts: ["image"], title: "Share image", ....}) // use info.srcUrl in onclick

If you use the above, then up to three menu items will be displayed (video, audio and image are mutually exclusive).