Closed
Bug 1473165
Opened 6 years ago
Closed 6 years ago
Figure out how to expose the main menubar (browser-menubar.inc) to an HTML document
Categories
(Firefox :: Menus, enhancement, P3)
Firefox
Menus
Tracking
()
RESOLVED
FIXED
People
(Reporter: bgrins, Unassigned)
References
Details
Attachments
(2 obsolete files)
We'd like to support top-level windows as HTML documents, so preprocessing in browser-menubar.inc isn't good for that.
There are a couple of general approaches we've thought of so far:
1) Parse out and append approximately the same markup, but do it from JS using a DOMParser (see MozXULElement.parseXULToFragment). This is an easier step, since we can even continue to use DTD files by including the entities in the string to be parsed).
2) Switch to a JS Menu API instead of using markup directly. Behind the scenes this would still generate XUL DOM (see for example https://searchfox.org/mozilla-central/rev/403038737ba75af3842ba6b43b6e2fb47eb06609/devtools/client/framework/menu.js#123). But it could also give a chance to refactor and improve how we do menus in the browser window (remove preprocessing, open up optimizations around not constructing the DOM or localizing until the menu items are going to be visible, etc).
Would like to do some more investigation to help figure out what the next steps are.
|
||
Updated•6 years ago
|
Priority: -- → P3
| Comment hidden (mozreview-request) |
|
Reporter | |
Comment 2•6 years ago
|
||
Pushed up a simpler alternative that would be less churn and would at least allow use from xhtml documents.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Reporter | |
Comment 9•6 years ago
|
||
The patches here actually mostly work, although various menu functionality will be broken (if the command relies on stuff that expects to be running in a XUL document such as various APIs on XULDocument, createElement assuming a XUL_NS, etc). The hidden window doesn't have very good test coverage to let us know what exactly is broken though, so I'm planning to prototype the main browser window as XHTML to figure out. This'll probably require a bunch of the work tracked in Bug 1453783.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Reporter | |
Updated•6 years ago
|
|
||
Comment 12•6 years ago
|
||
| mozreview-review | ||
Comment on attachment 8991142 [details]
Bug 1473165 - Migrate browser.xul to xhtml
https://reviewboard.mozilla.org/r/256110/#review269006
::: browser/base/content/browser.xhtml:34
(Diff revision 5)
> # hiddenWindow.xul.
> -<!DOCTYPE window [
> +<!DOCTYPE html [
> #include browser-doctype.inc
> ]>
>
> -<window id="main-window"
> +<html id="main-window"
Shouldn’t this be `html:html` as the default namespace is the XUL namespace, and last I checked, there’s no `html` element in XUL.
::: browser/base/content/browser.xhtml:75
(Diff revision 5)
> # All JS files which are needed by browser.xul and other top level windows to
> # support MacOS specific features *must* go into the global-scripts.inc file so
> # that they can be shared with macWindow.inc.xul.
> #include global-scripts.inc
>
> -<script type="application/javascript">
> +<script type="text/javascript" xmlns="http://www.w3.org/1999/xhtml">
This should be `application/javascript`, as `text/javascript` is obsolete, according to [RFC 4329](https://tools.ietf.org/html/rfc4329#section-7).
::: browser/base/content/global-scripts.inc:10
(Diff revision 5)
>
> # If you update this list, you may need to add a mapping within the following
> # file so that ESLint works correctly:
> # tools/lint/eslint/eslint-plugin-mozilla/lib/environments/browser-window.js
>
> -<script type="application/javascript">
> +<script type="text/javascript" xmlns="http://www.w3.org/1999/xhtml">
This should be `application/javascript`, as `text/javascript` is obsolete, according to [RFC 4329](https://tools.ietf.org/html/rfc4329#section-7).
|
|
Reporter | |
Comment 13•6 years ago
|
||
(In reply to ExE Boss from comment #12)
> Comment on attachment 8991142 [details]
> Bug 1473165 - Migrate browser.xul to xhtml
>
> https://reviewboard.mozilla.org/r/256110/#review269006
>
> ::: browser/base/content/browser.xhtml:34
> (Diff revision 5)
> > # hiddenWindow.xul.
> > -<!DOCTYPE window [
> > +<!DOCTYPE html [
> > #include browser-doctype.inc
> > ]>
> >
> > -<window id="main-window"
> > +<html id="main-window"
>
> Shouldn’t this be `html:html` as the default namespace is the XUL namespace,
> and last I checked, there’s no `html` element in XUL.
I know it is odd, but by allowing the HTML element to be XUL namespaced, selectors like `:root` defined in XUL namespaced CSS files get applied to it. It may end up causing other issues later on, but it seems to work good enough at least for testing.
|
||
Comment 14•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #13)
> (In reply to ExE Boss from comment #12)
> > Comment on attachment 8991142 [details]
> > Bug 1473165 - Migrate browser.xul to xhtml
> >
> > https://reviewboard.mozilla.org/r/256110/#review269006
> >
> > ::: browser/base/content/browser.xhtml:34
> > (Diff revision 5)
> > > # hiddenWindow.xul.
> > > -<!DOCTYPE window [
> > > +<!DOCTYPE html [
> > > #include browser-doctype.inc
> > > ]>
> > >
> > > -<window id="main-window"
> > > +<html id="main-window"
> >
> > Shouldn’t this be `html:html` as the default namespace is the XUL namespace,
> > and last I checked, there’s no `html` element in XUL.
>
> I know it is odd, but by allowing the HTML element to be XUL namespaced,
> selectors like `:root` defined in XUL namespaced CSS files get applied to
> it. It may end up causing other issues later on, but it seems to work good
> enough at least for testing.
Sounds like this deserves a comment explaining this.
|
|
Reporter | |
Comment 15•6 years ago
|
||
(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #14)
> > I know it is odd, but by allowing the HTML element to be XUL namespaced,
> > selectors like `:root` defined in XUL namespaced CSS files get applied to
> > it. It may end up causing other issues later on, but it seems to work good
> > enough at least for testing.
>
> Sounds like this deserves a comment explaining this.
Yeah, for sure. FYI we're also looking into if it'll be possible to actually land browser.xhtml as more-or-less `#include browser.xul` with a define for the root tag name. That would make it easier for us to run tests against it and break down / fix issues individually.
|
|
Reporter | |
Comment 16•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #15)
> FYI we're also looking into if it'll be possible to actually
> land browser.xhtml as more-or-less `#include browser.xul` with a define for
> the root tag name. That would make it easier for us to run tests against it
> and break down / fix issues individually.
Alright, this can now be done with `mk_add_options 'export MOZ_BROWSER_XHTML=1'` in .mozconfig. I'm going to obsolete the patches here but keep this bug open as I know some of the menu functionality isn't working in that build yet.
Depends on: 1482256
|
|
Reporter | |
Updated•6 years ago
|
Attachment #8991142 -
Attachment is obsolete: true
|
|
Reporter | |
Updated•6 years ago
|
Attachment #8991150 -
Attachment is obsolete: true
|
|
Reporter | |
Comment 17•6 years ago
|
||
Going to close this since it's generally working, and handle any menubar breakage in individual bugs.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•