AM v2 should port over the browser tabs feature (currently only works on Fennec, but will be expanded to Firefox OS). Might be good to think about this early on, just to ensure there is a reasonable place to put this in the UI.

I tried to port this feature. But it means we'll have to support 4 types of projects: packaged, hosted, runtimeApp, runtimeTab. It makes the code very confusing. So this will require some refactoring of how we handle projects.

This is important, but not used enough to block enabling webide.

This allows runtimes with tabs, such as the local runtime or Fennec over USB, to show the tabs in the project menu. You can debug the tabs as expected. Also, if you open or close tabs on the remote device, our local list is kept up to date. In order to move quickly here, I am opting to defer the project agnostic fix for now, and instead add a new type. I think we can live with it for now.

Comment on attachment 8480287 [details] [diff] [review] Show runtime tabs in WebIDE More types :) Thanks, this will be useful. Please make sure this survives tab closing and tab navigation (update the title/name in the UI). # browser/devtools/webide/content/webide.js >@@ -557,17 +558,18 @@ let UI = { > } else { > stopCmd.setAttribute("disabled", "true"); > debugCmd.setAttribute("disabled", "true"); > } > > // If connected and a project is selected > if (AppManager.selectedProject.type == "runtimeApp") { > playCmd.removeAttribute("disabled"); >- } else if (AppManager.selectedProject.type == "mainProcess") { >+ } else if (AppManager.selectedProject.type == "mainProcess" || >+ AppManager.selectedProject.type == "tab") { > playCmd.setAttribute("disabled", "true"); > stopCmd.setAttribute("disabled", "true"); > } else { > if (AppManager.selectedProject.errorsCount == 0) { > playCmd.removeAttribute("disabled"); > } else { > playCmd.setAttribute("disabled", "true"); > } That means we can't reload a tab by pressing play? >+ let tabs = AppManager.deviceStore.object.tabs; >+ for (let i = 0; i < tabs.length; i++) { >+ let tab = tabs[i]; >+ let panelItemNode = document.createElement("toolbarbutton"); >+ panelItemNode.className = "panel-item"; >+ panelItemNode.setAttribute("label", tab.title); The DOM Inspector (the addon) uses the title, not the URL. It's very annoying. Can we use the URL here? Or maybe the label could be `{domain}: {title}`? >+ panelItemNode.setAttribute("image", AppManager.DEFAULT_PROJECT_ICON); Let's not use any image here. Or maybe we could use the favicon. # browser/devtools/webide/modules/app-manager.js > isProjectRunning: function() { >- if (this.selectedProject.type == "mainProcess") { >+ if (this.selectedProject.type == "mainProcess" || >+ this.selectedProject.type == "tab") { > return true; > } What happens when the tab is closed?

Changes: * Allows reloading with play * Tracks tab navigation and close correctly TODO: * Change label to domain: title * Remove image * Tests

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2e899eebc59a

Comment on attachment 8481749 [details] [diff] [review] Show runtime tabs in WebIDE (v3) For the icon, maybe you can use this (client side): https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/places_favicon - not perfect, but it should work. Titles can be very long, and it messes up the UI (see previous comment). You can maybe use max-width for the panel, the title in the detail screen, and the panel button (title appears in this 3 locations). Maybe you want to use an even shorter name, like just the domain, or "Tab 3".

Comment on attachment 8481749 [details] [diff] [review] Show runtime tabs in WebIDE (v3) r- because of the UI issue mentioned above. browser/devtools/webide/content/webide.js >+ let url = new URL(tab.url); >+ // Wanted to use nsIFaviconService here, but it only works for visited >+ // tabs, so that's no help for any remote tabs. Maybe some favicon wizard >+ // knows how to get high-res favicons easily, or we could offer actor >+ // support for this. Support for ipod touch highres icons has landed some days ago. Bug 921014. But I think it's only for child browsers. Anyway, I think Margaret can help you. browser/devtools/webide/modules/app-manager.js b/browser/devtools/webide/modules/app-manager.js >+ // Wanted to use nsIFaviconService here, but it only works for visited >+ // tabs, so that's no help for any remote tabs. Maybe some favicon wizard >+ // knows how to get high-res favicons easily, or we could offer actor >+ // support for this. >+ tab.favicon = uri.prePath + "/favicon.ico"; Let me know if this works better: https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/places_favicon >+ tab.name = tab.title; tab.title can be `undefined`. Then the title should be just the domain. browser/devtools/webide/modules/tab-store.js >+ _selectedTab: null, >+ get selectedTab() { >+ return this._selectedTab; >+ }, I'm surprised to see this here. I would have expected to see this code on app-manager.js side. If there's 2 consumers of the same tab-store, there could be only one selection. I don't think this scenario would ever happen, but I would expect the store to not know about what tab is being debugged. You probably do that to be able to send the navigate/close events only for the tab being debugged. I would have sent these events for all the tabs and let the consumer of this store do the filtering. It's what we do for apps. But maybe your way is cleaner... and if so, we will need (later) to use the same logic for aps. You tell me.

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #8) > Comment on attachment 8481749 [details] [diff] [review] > Show runtime tabs in WebIDE (v3) > > For the icon, maybe you can use this (client side): > https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/ > places_favicon - not perfect, but it should work. I tried that actually... it unfortunately just calls nsIFaviconService, so it has the same flaws. > Titles can be very long, and it messes up the UI (see previous comment). You > can maybe use max-width for the panel, the title in the detail screen, and > the panel button (title appears in this 3 locations). Okay, I've used max-width / ellipsis styling in all these places. Let me know if the max-widths should be tweaked. (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #9) > browser/devtools/webide/content/webide.js > >+ let url = new URL(tab.url); > >+ // Wanted to use nsIFaviconService here, but it only works for visited > >+ // tabs, so that's no help for any remote tabs. Maybe some favicon wizard > >+ // knows how to get high-res favicons easily, or we could offer actor > >+ // support for this. > > Support for ipod touch highres icons has landed some days ago. Bug 921014. > But I think it's only for child browsers. > > Anyway, I think Margaret can help you. I've talked to a few people on #fx-team about this, and it seems like you can do much of anything intelligent unless you've loaded the full page to get the extra meta tags. Rather than have the client randomly fetch the page the server is viewing just to get meta tags, I think the most reasonable path is for us to support a favicon request on the server. Filed bug 1061654 and noted in the comments. > > >+ tab.name = tab.title; > > tab.title can be `undefined`. Then the title should be just the domain. I've changed this to use the text "Loading…" if the title is empty. So for http URLs, it momentarily shows "{domain}: Loading…". > browser/devtools/webide/modules/tab-store.js > >+ _selectedTab: null, > >+ get selectedTab() { > >+ return this._selectedTab; > >+ }, > > I'm surprised to see this here. I would have expected to see this code > on app-manager.js side. If there's 2 consumers of the same tab-store, > there could be only one selection. I don't think this scenario would > ever happen, but I would expect the store to not know about what tab > is being debugged. Yeah... I agree this is a bit odd. I think the right fix is for this work to be done in a TabProject (a specialized project object) which we make as part of the project-agnostic work. I've added a comment to this effect, but let me know if it bothers you enough for me to change right now. You're right that this code could fold back into app-manager.js, but I tried to avoid that because there's already so much stuff in there. So, the tab-store is currently an awkward in-between state since we don't have project-agnostic yet (the file might disappear entirely after that work). > You probably do that to be able to send the navigate/close events only > for the tab being debugged. I would have sent these events for all the tabs > and let the consumer of this store do the filtering. It's what we do for > apps. The tab navigation case is a bit different than apps, because we don't have the equivalent of "watchApps" for tabs. You are only told if a tab has navigated if you've already attached[1] to the tab. So, I made an explicit "select a tab" path to make sure the attaching happens when a tab is selected (since we currently only care about such events for a selected tab project). Other options are: * add a "watchTabs" path to the server * attach to all the tabs and filter the events but since we just only need one tab, it seems logical to only attach to it. I think the main issue is just "where does the code go?", and so that will improve with project-agnostic work. [1]: http://hg.mozilla.org/mozilla-central/annotate/c360f3d1c00d/toolkit/devtools/server/actors/webbrowser.js#l1993 > But maybe your way is cleaner... and if so, we will need (later) to use the > same > logic for aps. You tell me. I don't really think the current solution is clear per-se, but it's okay for now. I at least like that it's in a separate file from app-manager.js. Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c8e215cc4244

(In reply to J. Ryan Stinnett [:jryans] from comment #10) > (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from > comment #9) > > browser/devtools/webide/content/webide.js > > >+ let url = new URL(tab.url); > > >+ // Wanted to use nsIFaviconService here, but it only works for visited > > >+ // tabs, so that's no help for any remote tabs. Maybe some favicon wizard > > >+ // knows how to get high-res favicons easily, or we could offer actor > > >+ // support for this. > > > > Support for ipod touch highres icons has landed some days ago. Bug 921014. > > But I think it's only for child browsers. > > > > Anyway, I think Margaret can help you. > > I've talked to a few people on #fx-team about this, and it seems like you > can do much of anything intelligent unless you've loaded the full page to > get the extra meta tags. Rather than have the client randomly fetch the > page the server is viewing just to get meta tags, I think the most > reasonable path is for us to support a favicon request on the server. Filed > bug 1061654 and noted in the comments. s/you can do much/you *cannot* do much/

Comment on attachment 8482732 [details] [diff] [review] Show runtime tabs in WebIDE (v4) Great! > +#project-panel-button > .panel-button-label { > + max-width: 300px; > +} Too large. 150px looks good here.

Changed tab max-width to 150px. My test dies for some reason that I can't determine right now... Seems to be the second bug where this has happened to me lately. :( I filed bug 1062611 to fix it up, since we want to land this now. Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=79f5fcc3b33e