Stop shipping DOM Inspector in Firefox builds

VERIFIED FIXED in Firefox 3 beta4

Status

()

Firefox
Build Config
P1
normal
VERIFIED FIXED
11 years ago
8 years ago

People

(Reporter: mconnor, Assigned: mconnor)

Tracking

({relnote})

Trunk
Firefox 3 beta4
relnote
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
There's discussion at:

news://news.mozilla.org:119/mailman.1363.1146461172.6235.dev-apps-firefox@lists.mozilla.org

Short, high-level summary is that now that the binary bits can be shipped in Gecko we can ship a single chrome-only extension from AMO, and hopefully foster additional development out of sync from the mainline development.  Most users do not understand or use the DOM inspector, and those who do will be able to install the extension.

(The size win will help too!)
(Assignee)

Updated

11 years ago
Flags: blocking-firefox2+
Heartily supported from a UI perspective.

We might want to file followup bugs (unless they're filed already?) for adding panels to the installer to go and fetch some of these extensions based on user option selections at install time, but I think rstrong has mentioned that won't be do-able for Fx2.
Should there be a bug to distribute DOMi as an extension before we remove it from the installer? This would lessen the noise this change will cause and give us an answer to the question of "how do I install it then?".
(In reply to comment #2)
> Should there be a bug to distribute DOMi as an extension

bug 339174 has been filed, fwiw.

Updated

11 years ago
Depends on: 339174
Priority: -- → P1

Updated

11 years ago
Whiteboard: 181b1+
Blocks: 333160
(Assignee)

Updated

11 years ago
Whiteboard: 181b1+ → no patch yet, 181b1+
Can someone write in here how the upgrade experience will work for people who already have DOMi installed in Fx 1.5, and then upgrade to Fx2 (with or without going via Fx 2a3 or b1 or whatever)?  I don't know what the choices are here, given start page, new installer, app-managed extensions, first-start compat checks and whatnot.
The installer and software update have a removed-files file that is parsed for files to remove. If used to remove the old DOMi this would also remove installs of DOMi that were added to the appDir's extensions directory by the user. A small subset to be sure - just making it clear that we have no way currently of differentiating them for the installer and software update.

The only difference with extensions that are appManaged is that they don't check for updates. There is no help or hurt here since users often don't have write access to the appDir's extensions directory.

The extension compatibility check on upgrade doesn't get us anywhere here as well.

The start page is something mconnor brought up iirc. I would think this would have to be "in your face" for people to notice it.

For software update we could keep DOMi in the mar file but as you stated the target audience tends to manually update - I believe you stated before the mar was available.

Comment 6

11 years ago
Could the 2.0 mar/installer just take the appManaged flag out of install.rdf and remove all other DOMI files? That would make the "incompatible extensions" wizard offer a DOMI from AMO and install it in the user's profile, right?
(In reply to comment #6)
> Could the 2.0 mar/installer just take the appManaged flag out of install.rdf
> and remove all other DOMI files? That would make the "incompatible extensions"
> wizard offer a DOMI from AMO and install it in the user's profile, right?
I believe this is possible. We would just replace the install.rdf with the installer and add an empty chrome.manifest. The EM won't check for updates to an extension if the user doesn't have write access to the current install location though so a subset of people wouldn't get the update.
If an extension is installed in both appdir and profile, does the profile one "win"?  If so, it would be nice for the EM to check profile as well as appdir for write access, and perform the update dance if the user can write to either.

That said, having this fail for the set of users who don't have appdir write access is something I can live with, since they'll have a hard time doing the install in the first place.
(In reply to comment #8)
> If an extension is installed in both appdir and profile, does the profile one
> "win"?  If so, it would be nice for the EM to check profile as well as appdir
> for write access, and perform the update dance if the user can write to either.
The profile one "wins".
All install locations are checked for write access.
Only active extensions are updated since they are the only ones visible.

Comment 10

11 years ago
Robert/Ben - any chance on a patch for b1?
No. I already told beltzconnor that this should wait for b2. The update issues are just too complicated.

Updated

11 years ago
Whiteboard: no patch yet, 181b1+ → no patch yet
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
(Assignee)

Comment 12

11 years ago
We need to move forward on this, since if we don't, bug 333160 needs a much bigger fix.

There are some open questions about how we're going to achieve this, but we need to solve the technical questions around upgrading users and just make this happen.
Whiteboard: no patch yet → no patch yet [at risk]
According to today's triage meeting, we are going to continue shipping DOMI as an app-managed extension with the app, but are also going to aggressively develop it (or Firebug) for manual install. This decision will be revisited for FF3.
Flags: blocking-firefox2+
Priority: P1 → P3
Whiteboard: no patch yet [at risk]
Target Milestone: Firefox 2 beta2 → Firefox 3 alpha1
Version: 2.0 Branch → Trunk
Component: General → Build Config
QA Contact: general → build.config
No longer blocks: 333160
Assignee: benjamin → nobody
(Assignee)

Comment 14

10 years ago
Taking, targeting.  More on this soon.
Assignee: nobody → mconnor
Flags: blocking-firefox3+
Target Milestone: Firefox 3 alpha1 → Firefox 3 M9
(Assignee)

Updated

10 years ago
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Depends on: 271812
(Assignee)

Updated

10 years ago
Priority: P3 → P2

Comment 15

10 years ago
In 3b1 we have (in my opinion) the worst of all worlds for DOMI. 
   Its an install-only option, visible only if you dig: people who need it lose.
   Its the only install option, cause millions of non-users to ponder "custom" for no point.

Was the option of simply including it always and eliminating custom considered? 
  -- Firebug users win: no "reinstall to get DOMi" instructions
  -- All users win installing: no custom, just go for it!
  -- A few curious users will see a tool they don't recognize and learn something.

John.
(In reply to comment #15)
> Was the option of simply including it always and eliminating custom considered? 
No, and as the acting owner of DOMi I don't want it to ship by default with Firefox.

>   -- Firebug users win: no "reinstall to get DOMi" instructions
It shouldn't matter - you use the binary components of it (last I checked), which are part of the layout, not part of the add-on.  This was only an issue in 1.5, and in 2.0 the components were moved to layout so it was always there.  I do not foresee this changing anytime soon.

Note to Mike:  Make sure the binary bits stay enabled in layout when dom inspector is disabled.

>   -- All users win installing: no custom, just go for it!
Except everyone who doesn't want it, they lose.

>   -- A few curious users will see a tool they don't recognize and learn
> something.
That's what AMO is for.

Comment 17

10 years ago
Shawn,

As far as I can tell, DOMi is GPLd.  As such, any owner of it (if indeed you are) has no right to tell people that they can't distribute it.

That said, I don't quite understand:
> >   -- A few curious users will see a tool they don't recognize and learn
> > something.
> That's what AMO is for.

... given that you can't download the Firefox DOMi from AMO at the moment.  Shall I add it?
> As far as I can tell, DOMi is GPLd.  As such, any owner of it (if indeed you
> are) has no right to tell people that they can't distribute it.

What does that have to do with whether or not it's included with Firefox? The source is out there, which is all the GPL mandates. It doesn't say how or where you have to distribute it. All the GPL mandates is that the code has to be available so anyone can build and distribute it, which this bug will have no effect on.

> ... given that you can't download the Firefox DOMi from AMO at the moment. 
> Shall I add it?
> 

Did you look at this bug's dependencies? Bug 271812 would be a good start...

Comment 19

10 years ago
My Comment #15 was motivated by simplifying the FF3 install. Automatically including DOMi as a solution was based on the Firebug dependency. Since that no longer seems to be the case, and since Shawn does not want DOMi in FF3, removing the custom install and not shipping DOMi makes even more sense. (Well unless its too late).

According to the recent comments here and on Bug 271812 it appears that the core problem is that we don't have enough resources to maintain DOMi as a multi-app, multi-version AddOn so it has to stay in the main FF build.
  
However, it seems as if maintaining DOMi as a XPI for FF3 is feasible.  Thunderbird and similar apps can continue to ship DOMi. DOMi is already available for FF2 and earlier. If custom install is removed from FF3, only FF3 users of DOMI suffer, and for them an AddOn version of DOMi can be built.

Hope this helps,
John.



(In reply to comment #19)
> According to the recent comments here and on Bug 271812 it appears that the
> core problem is that we don't have enough resources to maintain DOMi as a
> multi-app, multi-version AddOn so it has to stay in the main FF build.
That is not 100% accurate.  It really is only the case for branch.  For 1.9, DOMi will be a multiple app package.
(Assignee)

Comment 21

10 years ago
* Binary pieces are installed regardless (I believe this is true for FF2 as well, but not FF1.5).

* DOMI is less approachable/ than Firebug and we want to make Firebug our primary developer tool, and it doesn't make a lot of sense for most users by default.

* For chrome developers, DOMI should be either easily buildable or installable, the UI doesn't really change, so putting a version on AMO is fairly easy.
(In reply to comment #21)
> * For chrome developers, DOMI should be either easily buildable or installable,
> the UI doesn't really change, so putting a version on AMO is fairly easy.
That isn't entirely correct.  The UI from 1.8.1 to 1.9 has several UI enhancements.  The bonus to not shipping it with Firefox is that we can modify the UI independent of the browser's ship dates and requirements.
(Assignee)

Comment 23

10 years ago
Created attachment 292636 [details] [diff] [review]
remove inspector from the default build
Attachment #292636 - Flags: review?(benjamin)
(Assignee)

Updated

10 years ago
Priority: P2 → P1

Comment 24

10 years ago
So is Shawn / someone else going to put DOMI on AMO then?
Yes - I will have DOMi up for 1.9 hopefully by b3.
Comment on attachment 292636 [details] [diff] [review]
remove inspector from the default build

>Index: browser/installer/windows/packages-static

>-bin\extensions\inspector@mozilla.org\platform\WINNT\chrome\icons\default\winInspectorMain.ico
>+
>+
>+
>+
>+
>+

Remove extra lines please.

>Index: browser/installer/removed-files.in

>+#Remove Inspector (present from upgrades from Fx2/Fx3b1)
>+extensions/inspector@mozilla.org/install.rdf
>+extensions/inspector@mozilla.org/components/inspector-cmdline.js
>+extensions/inspector@mozilla.org/chrome.manifest
>+extensions/inspector@mozilla.org/chrome/inspector.jar
>+extensions/inspector@mozilla.org/defaults/preferences/inspector.js
>+extensions/inspector@mozilla.org/platform/WINNT/chrome/icons/default/winInspectorMain.ico
>+components/inspector.xpt

Please put this hunk next to the existing code that removes old inspector files:
http://mxr.mozilla.org/mozilla/source/browser/installer/removed-files.in#91
Attachment #292636 - Flags: review?(benjamin) → review+
What exactly is in components/inspector.xpt?  I'm worried that the binary components that ship with layout might be defined in that (and firebug, among others, use those).
Indeed, inspector.xpt comes from http://lxr.mozilla.org/seamonkey/source/layout/inspector/public/Makefile.in#47 
I'm not sure we want to remove that then, unless we expect anyone who wants to use that code to have to generate that and ship it themselves.
inspector.xpt should not be in the build AFAIK. At packaging time it will be linked into the master .xpt file (probably browser.xpt). I believe it is correct to remove it, and may be necessary. Testing encouraged, of course.
Being able to look into the innards of Firefox out of the box seems quite appealing to me. I think it's a little bit like what "view source" is to the Web.
Most users do not understand or use "view source", and those who do... you get the point? It's not only useful as a daily tool for existing authors and developers, it's also an entry point.

I'm not sure if I'd be here if there would have been no DOM Inspector by default. So I hope you know the full impact of what you're doing. Being able to update DOMi independently from the host app seems secondary to me; in the long run DOMi could even suffer from that.
Duplicate of this bug: 410447
(Assignee)

Comment 33

9 years ago
Lost my cleaned up patch with my hard drive, will get this cleaned up later today.  I know there's a tradeoff to not providing a built in ladder to the dark dark attic of our chrome code, but I think that our developer:user ratio is beyond belief, so it feels like something we can do at this point, and balance that out somewhere else...
Whiteboard: [needs cleaned up patch for checkin]
(Assignee)

Updated

9 years ago
Target Milestone: Firefox 3 beta2 → Firefox 3 beta4
(Assignee)

Comment 34

9 years ago
Created attachment 305613 [details] [diff] [review]
review comments addressed and unbitrotted
Attachment #292636 - Attachment is obsolete: true
(Assignee)

Comment 35

9 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs cleaned up patch for checkin]

Comment 36

9 years ago
Still need to clean up a couple of installer files:
http://mxr.mozilla.org/seamonkey/search?string=inspector&find=browser%2Finstaller&findi=&filter=&tree=seamonkey

Comment 37

9 years ago
Is it correctly understood that the DOM Inspector will be available as a extension instead?
(In reply to comment #37)
> Is it correctly understood that the DOM Inspector will be available as a
> extension instead?
Yes - see bug 271812

Comment 39

9 years ago
Um, I know you hate bug spam and all. But would it be too much to ask that you put DOMi back until after you have the freestanding extension on AMO? There are a bunch of us themers who need it just at the moment, since a bunch of theme-related bugfixes just landed.

Comment 40

9 years ago
Ed, This should also work for Minefield:

<http://forums.mozillazine.org/viewtopic.php?t=629027>
"Getting a working DOM Inspector in Thunderbird 3.0a trunk."
(In reply to comment #39)
> Um, I know you hate bug spam and all. But would it be too much to ask that you
> put DOMi back until after you have the freestanding extension on AMO? There are
> a bunch of us themers who need it just at the moment, since a bunch of
> theme-related bugfixes just landed.
There's also an xpi attached to bug 271812.  That bug has only been mentioned several times now, yet people still keep commenting about the same issues... 
This looks fixed for Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre.   however, we will need to retest this during major update from 2.x to 3.0b4+
Depends on: 420028

Updated

9 years ago
Keywords: relnote
(In reply to comment #36)
> Still need to clean up a couple of installer files:
> http://mxr.mozilla.org/seamonkey/search?string=inspector&find=browser%2Finstaller&findi=&filter=&tree=seamonkey
> 
Steffen, Shawn - 
Wait a sec..  if we still need cleanup code on this bug, then should we create a spin off bug on this work, and mark this bug a dependency of that bug?   If its not a dependency, then we can really mark this bug as fixed.
At this point I'd say file a new bug.
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4) Gecko/2008030317 Firefox/3.0b4.   Updates also remove Dom Inspector.

steffen, shawn, please feel free to open a new bug for the code cleanup issue.
Status: RESOLVED → VERIFIED

Updated

9 years ago
Depends on: 450164
You need to log in before you can comment on or make changes to this bug.