Evan, Thanks for the detailed feedback. We want the extension system to be very easy to use, but there are still some sharp edges that need smoothing out. Having people that are totally new to the system try it out and write down detailed impressions helps to know which things most need focusing on.
Some of the things you pointed out are either already fixed on trunk, or have code reviews pending. Others are things that we agree need doing, but don't have prioritized. To help understand the issues you faced, I tried debugging your extension a bit to fix the flakiness. I made some minor adjustments and it seems to be working well now. I also packed it up (attached). I've sent the pem file to you separately. Detailed responses inline... On Sat, Sep 12, 2009 at 7:02 PM, Evan Martin <[email protected]> wrote: > > [resend, I think I screwed up the previous three tries] > I wrote an extension that adds a page action to trigger Readability: > http://lab.arc90.com/experiments/readability/ > > It's basically just a glorified bookmarklet. > > Code is here: > http://neugierig.org/software/git/?url=chrome-readability/ > Browse it here: > http://neugierig.org/software/git/?url=chrome-readability/tree/ > > I can't provide a .crx because I wasn't able to figure out how to > build one, which I think means I can't actually install it. :~( Packaging isn't implemented yet on linux, but it is scheduled for the beta milestone. > Here's some feedback on the process. I know extensions are still > under development, and that surely most if not all of these are > already known bugs, and that others are probably my fault for doing it > on Linux. I thought it would still be helpful to give an overview of > points of confusion I ran into, in case any of these aren't yet known > bugs. It's really useful feedback, even in cases where they are known bugs. Having multiple people hit them lets us know where to focus. > - Weight. > This feels like a *lot* of code (content script, page action, > background page, manifest, message ports) just to make a bookmarklet > appear in the URL bar. I wonder if there's a place for a "simple" > extension API for bookmarklet-y sorts of things? The majority of the complexity here is setting up the content script and the messaging to it. We have a code review in progress now that adds a programmatic script execution function to extensions. So from your background page, you could do something like: chrome.tabs.executeScript({source: "config variables..."}); chrome.tabs.insertCSS({file: "readability/readability.css"}); chrome.tabs.insertCSS({file: "readability/readability-print.css"}); chrome.tabs.executeScript({ file: "readability/readability-print.js"}); With that, it would just be the manifest, hooking the page action events, and then running this script. This seems like about the right amount of complexity to me, without having to add any special cases. > - Making my page action show up. > It wasn't clear to me how to make my action just always show up. > I think I may have done it wrong: > http://neugierig.org/software/git/?url=chrome-readability/tree/background.html > since it feels unreliable (sometimes it doesn't show). A couple things here: - There was a bug in the page actions system that made them flaky. This has been fixed on trunk. - It is easy to mess up the page action code because the tabs events are hard to use. You had a bug in yours that I've fixed (see attached patch), but I wouldn't expect developers to get this right. I've filed a bug to make an easier to event. - There is a really nasty bug outstanding that makes content scripts get GC'd if they don't use any DOM events. I hadn't really internalized how nasty until I tried to debug your extension, so I've upped the priority of that bug. > - The failure modes are confusing. > Sometimes it prints to the console (when I've made a typo in my > manifest); other times it prints to the error console of the extension > (bugs in my background js); other times it prints to the page's error > console (bugs in my content script). Many of those times there's no > obvious way to map the error back to the line that is failing. I agree there should be a centralized extension error console. We actually had something like this at one point, but it wasn't working well (you actually reported a bug about this -- "useless errors on chrome://extensions/") -- so I removed it as a stop-gap. I agree we should bring it back, but better. All the errors should be printing their line numbers, except for errors from asynchronous APIs, which should print the name of the function you called (We could even get the line numbers of those errors, but it doesn't come for free). If you are seeing a case where this isn't happening, please give details. > - JS console. > Do we really have no UI to get to the JS console? I had to open the > developer tools, then guess that one of the icons at the bottom of > the window would show me the messages. I see this has already be answered. > - The docs around content scripts communicating with the embedding > page aren't too clear. See e.g.: > http://code.google.com/chrome/extensions/content_scripts.html#messaging > That section is mostly just a big example but for example nowhere is > the postMessage API described. I'd prefer it to be laid out more > like: > - how to make each endpoint listen for messages > - how to make each endpoint send a message That is a good idea for how that section should be structured. I think Kathy is listening in, so I'll leave this to her. > - Doc organization. > It would've been clearer to me if there is one more level of nesting. > Sections like toolstrips, page actions are features with manifest > edits as well as APIs, while sections like tabs and windows are just > API references. I agree. I think that from a developer's point of view, you are just trying to make page actions work. Whether a piece of information is "reference" or "overview" or "manifest" is not really relevant to you. You just want to know how the whole thing works. This is something we've gone around a bit on the team. We'll keep working to refine the docs structure over time. > - Building the .crx. > strace -fo log chromium-browser --user-data-dir=/home/martine/test > --pack-extension=`pwd`/readext > --pack-extension-key=chrome-readability.pem > Doesn't show it ever trying to create my .pem. Maybe it's not implemented, > but it'd be nice if it at least complained in that case. I see you've submitted a patch to make it complain. Thanks! - a --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
chrome-readability.crx
Description: Binary data
0001-meh.patch
Description: Binary data
