1417664 - Package Layout Debugger interface for use with debug artifact builds

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Layout debugger is almost usable in debug artifact builds, but the XPCOM interface is not packaged, the debugger UI throws an error on use.

Adding the .xpt to the package manifest should solve this.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1417664 - Include layout debugger interface for packaged builds.

Review commit: https://reviewboard.mozilla.org/r/199970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199970/

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 8928710 [details]
Bug 1417664 - Include layout debugger interface for packaged builds.

https://reviewboard.mozilla.org/r/199970/#review205110

I think it would be good to include this in DEBUG artifact builds,
but I'm not familiar with packaging at all so you should probably
also ask someone who is for an additional review (unless you feel
familiar enough with it yourself).

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8928710 [details]
Bug 1417664 - Include layout debugger interface for packaged builds.

https://reviewboard.mozilla.org/r/199970/#review205110

I've fixed missing XPCOM interfaces with similar changes in the past, so I believe the approach is correct...

But just to be sure, I'll also check with a build peer.

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8928710 [details]
Bug 1417664 - Include layout debugger interface for packaged builds.

https://reviewboard.mozilla.org/r/199970/#review205136

Does this not make sense on Fennec?  Perhaps it's always the host (Desktop) debugging client who would use the XPT?  In any case, this looks fine.

::: browser/installer/package-manifest.in:249
(Diff revision 1)
>  @RESPATH@/components/jar.xpt
>  @RESPATH@/components/jsdebugger.xpt
>  @RESPATH@/components/jsdownloads.xpt
>  @RESPATH@/browser/components/jsinspector.xpt
>  @RESPATH@/components/layout_base.xpt
> +#ifdef MOZ_DEBUG

y u no https://searchfox.org/mozilla-central/source/mobile/android/installer/package-manifest.in#173?

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8928710 [details]
Bug 1417664 - Include layout debugger interface for packaged builds.

https://reviewboard.mozilla.org/r/199970/#review205136

> y u no https://searchfox.org/mozilla-central/source/mobile/android/installer/package-manifest.in#173?

Hmm, at the moment, the layout debugger UI that uses this XPCOM interface opens as a separate window in Firefox desktop via a browser.xul overlay:

https://searchfox.org/mozilla-central/source/layout/tools/layout-debug/ui/content/layoutdebug-overlay.xul

so there's not really a Fennec equivalent I don't think...

However, we do still appear to package and ship the code behind the interface for Fennec debug builds...  So, I think just for general sanity, I'll add it to Fennec as well.  (We may want to decide separately to not build any of this for Fennec, but I'll leave that for another bug.)

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8928710 [details]
Bug 1417664 - Include layout debugger interface for packaged builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199970/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07d02e9d114c
Include layout debugger interface for packaged builds. r=mats,nalexander

Comment 8

[Mats Palmgren (inactive)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> Hmm, at the moment, the layout debugger UI that uses this XPCOM interface
> opens as a separate window in Firefox desktop...

FYI, you can also run it from the command line with -layoutdebug,
then you get only that one window.

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/07d02e9d114c

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)

<snip>

> I'll add it to Fennec as well.  (We may want to decide separately to not
> build any of this for Fennec, but I'll leave that for another bug.)

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1417978 to track removing this from Android as appropriate.