[geckoview] Allow to set chromeURI from Java

RESOLVED FIXED in Firefox 50

Status

()

Firefox for Android
GeckoView
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

unspecified
Firefox 50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Right now, `widget/android/nsWindow.cpp` always takes Gecko's defaultChromeURI pref.  That's not very GeckoView friendly; this ticket tracks adding a parameter to allow loading a different chromeURI from GeckoView (or a GeckoView consumer).

The intention is that GV consumers will not use browser.xul, but instead geckoview.xul; and that they'll provide content URLs from their assets/ directories.  This lets them do things without modifying the omnijar and especially Gecko's prefs *before* Gecko startup.
(Assignee)

Comment 1

2 years ago
Created attachment 8764731 [details]
Bug 1281890 - Allow setting chromeURI from Java.

The intention is that GeckoView consumers will not use browser.xul,
but instead geckoview.xul; and that they'll provide content URLs from
their assets/ directories.  This lets them do things without modifying
the omnijar and in particular without setting Gecko's prefs *before*
Gecko startup.

Review commit: https://reviewboard.mozilla.org/r/60424/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60424/
Attachment #8764731 - Flags: review?(snorp)
Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,

<gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />
(Assignee)

Comment 3

2 years ago
(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> 
> <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />

Later, yes:  I removed the attrs.xml for GeckoView to ease separating the two parts of the code.
https://reviewboard.mozilla.org/r/60424/#review59094

This is great, thanks.

::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java:1790
(Diff revision 1)
> +         * XUL container <tt>chrome://browser/content/geckoview.xul</tt>.  See
> +         * <a href="https://developer.mozilla.org/en/docs/toolkit.defaultChromeURI">https://developer.mozilla.org/en/docs/toolkit.defaultChromeURI</a>
> +         *
> +         * @return URI or null.
> +         */
> +        String getDefaultChromeURI();

Thanks for documenting this, maybe it will start a trend.
(In reply to Nick Alexander :nalexander from comment #3)
> (In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> > Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> > 
> > <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />
> 
> Later, yes:  I removed the attrs.xml for GeckoView to ease separating the
> two parts of the code.

IMO we shouldn't be adding more stuff to GeckoInterface, which has become a big dumping ground for unrelated things. That's why I'd much prefer an attribute for setting the chrome URI.

Also, GeckoView had mGeckoEventListener and mNativeEventListener to prevent exposing the "handleMessage" methods as public methods of GeckoView. Only actual APIs of GeckoView should be public.

Lastly, for the most part GeckoView.xul doesn't need to send "Gecko:Ready". We should get rid of GeckoView's remaining dependency on the "Gecko:Ready" message soon.
(Assignee)

Comment 7

2 years ago
(In reply to Jim Chen [:jchen] [:darchons] from comment #6)
> (In reply to Nick Alexander :nalexander from comment #3)
> > (In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> > > Maybe GeckoView can accept a chrome_uri attribute in the layout file? e.g.,
> > > 
> > > <gecko:GeckoView gecko:chrome_uri="chrome://browser/content/geckoview.xul" />
> > 
> > Later, yes:  I removed the attrs.xml for GeckoView to ease separating the
> > two parts of the code.
> 
> IMO we shouldn't be adding more stuff to GeckoInterface, which has become a
> big dumping ground for unrelated things. That's why I'd much prefer an
> attribute for setting the chrome URI.

I agree, but I'm not willing to block GV progress on getting all the exposed APIs correct.  GV needs to fly free of Fennec, and pushing stuff into GI and cleaning up later is the fastest way I see to cutting those cords.  If you or somebody else is willing to split GI, please do, but it's going to be hard to figure out what the GV interface should look like whilst it's still bolted to the belly of Fennec.

> Also, GeckoView had mGeckoEventListener and mNativeEventListener to prevent
> exposing the "handleMessage" methods as public methods of GeckoView. Only
> actual APIs of GeckoView should be public.

Agree.

> Lastly, for the most part GeckoView.xul doesn't need to send "Gecko:Ready".
> We should get rid of GeckoView's remaining dependency on the "Gecko:Ready"
> message soon.

I agree, but I didn't know this when I started :)

Comment 8

2 years ago
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f08130ec8dc1
Allow setting chromeURI from Java. r=snorp

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f08130ec8dc1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.