If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Migrate inspector.xul to HTML

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Inspector
P1
enhancement
VERIFIED FIXED
2 years ago
11 months ago

People

(Reporter: pbro, Assigned: Honza)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 verified)

Details

(Whiteboard: [devtools-html])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Once the most important parts of the inspector have been migrated to HTML (bug 1259812, bug 1259819, bug 1260552), we should make inspector.xul be inspector.xhtml instead and migrate the remaining elements to HTML, like the toolbar and search field.

This also depends on contextmenus and panels being converted to HTML equivalent too (bug 1259834, bug 1257613).
Depends on: 1260493
(Reporter)

Updated

2 years ago
Severity: normal → enhancement
Alias: devtools-html-inspector
No longer blocks: 1259121
Whiteboard: [meta]
Keywords: meta
(Reporter)

Comment 1

a year ago
Brian, shouldn't this be in devtools.html track 2 MVP? I think we might have missed it during breakdown.
Flags: needinfo?(bgrinstead)
(In reply to Patrick Brosset <:pbro> from comment #1)
> Brian, shouldn't this be in devtools.html track 2 MVP? I think we might have
> missed it during breakdown.

In some sense this is the end goal of all of the work between tracks 1 and 2.  I agree we should be tracking it - and I guess track 2 is the logical place to put it.  I think we can give it a try once bug 1259819 and bug 1260552 are fixed.
Flags: needinfo?(bgrinstead)
Making this not a meta bug, this can be a work bug for converting inspector.xul to inspector.xhtml
Blocks: 1263750
Keywords: meta
Whiteboard: [meta] → [devtools-html][triage]
Alias: devtools-html-inspector
Whiteboard: [devtools-html][triage] → [devtools-html]
(Reporter)

Comment 4

a year ago
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
Priority: P2 → --

Comment 5

a year ago
What is the difference between this bug and bug 1271127 ?
Flags: qe-verify?
Priority: -- → P2
(In reply to David Bruant from comment #5)
> What is the difference between this bug and bug 1271127 ?

No difference, thanks for point that out.
Duplicate of this bug: 1271127
Blocks: 1289464
Flags: qe-verify? → qe-verify+
QA Contact: petruta.rasa
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Iteration: --- → 51.1 - Aug 15
Priority: P2 → P1
After discussing with Honza, the inspector.xul is being migrated to XHTML as part of Bug 1260552. We will just check if anything remains to be done after it lands.
Iteration: 51.1 - Aug 15 → 51.2 - Aug 29
Status: ASSIGNED → NEW
Iteration: 51.2 - Aug 29 → ---
Priority: P1 → P2
(Assignee)

Updated

a year ago
Duplicate of this bug: 1302383
(Assignee)

Comment 10

a year ago
Created attachment 8790660 [details] [diff] [review]
bug1262443.patch

I was experimenting with this a bit and here is preliminary patch.
It's based on patches from bug 1260552 and a patch from bug 1294186.

Honza
Assignee: jdescottes → odvarko
Status: NEW → ASSIGNED
(Assignee)

Comment 11

a year ago
Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=59c75ae3c64d

Test failures are mostly related to CSS property editing (Rule View) and clipboard (cut/copy/paste context menu items). A lot of failures might be related to wrong focus management.

Honza
Iteration: --- → 51.3 - Sep 19
Priority: P2 → P1
(Assignee)

Comment 12

a year ago
Created attachment 8791891 [details] [diff] [review]
bug1262443.patch

Patch updated.

It depends on patch from bug 1294186

Honza
Attachment #8790660 - Attachment is obsolete: true
Attachment #8791891 - Flags: review?(pbrosset)
(Assignee)

Comment 13

a year ago
Created attachment 8791894 [details] [diff] [review]
bug1262443-tests.patch

Fixing tests.

Honza
Attachment #8791894 - Flags: review?(pbrosset)
(Assignee)

Comment 14

a year ago
Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f57981f2a52

There are some oranges, but I believe that they are unrelated.

Honza
(Reporter)

Comment 15

a year ago
Comment on attachment 8791891 [details] [diff] [review]
bug1262443.patch

Review of attachment 8791891 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like you deleted inspector.xul and created inspector.xhtml. Could you use 'move' instead? So that history is preserved?
Looks good otherwise.

::: devtools/client/inspector/inspector.xhtml
@@ +28,5 @@
> +          src="chrome://devtools/content/shared/theme-switching.js"></script>
> +</head>
> +
> +<body class="theme-body devtools-monospace" role="application">
> +  <html:div class="inspector-responsive-container theme-body inspector">

Can we not get rid of the all the html namespaces throughout the file now?
Attachment #8791891 - Flags: review?(pbrosset) → review+
(Reporter)

Comment 16

a year ago
Comment on attachment 8791894 [details] [diff] [review]
bug1262443-tests.patch

Review of attachment 8791894 [details] [diff] [review]:
-----------------------------------------------------------------

Simple test changes with comments, works for me!
Attachment #8791894 - Flags: review?(pbrosset) → review+
(Assignee)

Comment 17

a year ago
Created attachment 8792918 [details] [diff] [review]
bug1262443.patch

(In reply to Patrick Brosset <:pbro> from comment #15)
> Comment on attachment 8791891 [details] [diff] [review]
> bug1262443.patch
> 
> Review of attachment 8791891 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It looks like you deleted inspector.xul and created inspector.xhtml. Could
> you use 'move' instead? So that history is preserved?
Done

> Looks good otherwise.
> 
> ::: devtools/client/inspector/inspector.xhtml
> @@ +28,5 @@
> > +          src="chrome://devtools/content/shared/theme-switching.js"></script>
> > +</head>
> > +
> > +<body class="theme-body devtools-monospace" role="application">
> > +  <html:div class="inspector-responsive-container theme-body inspector">
> 
> Can we not get rid of the all the html namespaces throughout the file now?
Yes, I'll do it in separate bug (bug 1304061) just to keep this patch as simple as possible.

I am going to include both patches from this bug into bug 1260552 so, we don't mix XUL & XHTML. It makes tests more stable (patch from bug 1260552 was backed out).

Also, the patch here doesn't depend on bug 1294186.

Let's keep this bug open for verification.

Thanks for the review Patrick!

Honza
Attachment #8791891 - Attachment is obsolete: true
Attachment #8792918 - Flags: review+
Iteration: 51.3 - Sep 19 → 52.1 - Oct 3

Comment 18

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/da144bd5bc51
Rename inspector.xul to inspector.xhtml; r=pbro
https://hg.mozilla.org/integration/fx-team/rev/36373fbb7811
Fix tests; r=pbro

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/da144bd5bc51
https://hg.mozilla.org/mozilla-central/rev/36373fbb7811
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Depends on: 1304727
Depends on: 1304732
Depends on: 1305007
Depends on: 1305051
Depends on: 1305056
No longer depends on: 1305056
Depends on: 1305056
Depends on: 1305503
I ran an overall check-up, no issues regressed.Also I verified all it's dependencies. I will mark this bug as verified.

Cheers!
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
Flags: qe-verify+
See Also: → bug 1316296
You need to log in before you can comment on or make changes to this bug.