Closed
Bug 1493446
Opened 6 years ago
Closed 6 years ago
Create a skeleton of the new internal settings HTML page
Categories
(Toolkit :: Preferences, enhancement, P1)
Toolkit
Preferences
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: Paolo, Assigned: vcote)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
This bug will add XHTML, JS, and CSS files for the new internal settings page, including the skeleton of a browser-chrome test for the page.
Assignee | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
I commented in Phabricator, but I think this bug should be blocked by bug 1486936.
Bug 1486936 will port the current about:config to Fluent, at that point you'll be able to reuse existing strings (where suitable), and add/remove others as work progresses.
Comment 3•6 years ago
|
||
Is there any reason not to use HTML directly instead of XHTML?
Reporter | ||
Comment 4•6 years ago
|
||
Thanks Francesco for your feedback, I wasn't aware that bug 1486936 was being worked on, and I suggested to Vincent that we could place our new files alongside the current "about:config" implementation in Toolkit.
Thinking this over, that probably wasn't the best option for the file layout, given the nature of our project. Based on the strengths and interests of the student team that emerged during our kick-off meeting yesterday, I made an early choice to treat this as a self-contained new implementation, and not a 1:1 port of the current "about:config". This will allow us to leverage the best of web technologies and leave behind all of the legacy deriving from the limitations of XUL widgets at the time the original "about:config" was written. One example above all is the lack of inline editing.
Given this, what we'll probably end up doing instead is to create a new component under "browser/components", and configure it so that it's built in Nightly only. I can see various advantages compared to an implementation in Toolkit:
1. We can more easily make the page feel like Firefox, in fact we're already using elements of Photon in our mockups.
2. We can encapsulate the files in an independent folder structure, and still reference the Photon visual artifacts.
3. We can defer implementing features like blocking the page by policy, since this won't ride the trains until ready.
4. We can defer support for third-party applications that are not Firefox, reducing scope and increasing success chances.
In addition to engineering mentoring, we're also engaging with the UX team, but at the same we don't want this project to draw too much time from other Firefox design tasks. We will start with a basic design based on common in-content styles, and then iterate from there in a lightweight manner.
Part of the work will be the definition of the release criteria. This will involve sign-off from UX and engineering to ride the trains and replace "about:config". We aim to do this as part of the project timeline, but we realize that the release criteria is not defined by us alone, and we may need additional time to address feedback.
While in Firefox we can stop building the current "about:config" sooner, we can treat the removal of the current Toolkit code as a separate step. The removal of this code from mozilla-central is mainly motivated by the presence of <tree>, so the timeline will be mostly linked to the <tree> removal.
Reporter | ||
Comment 5•6 years ago
|
||
(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #3)
> Is there any reason not to use HTML directly instead of XHTML?
We're using XHTML for most of out in-content pages, and also for the new "browser.xhtml".
https://searchfox.org/mozilla-central/search?q=.xhtml
Reporter | ||
Comment 6•6 years ago
|
||
Even though we don't plan to use XUL elements there, there's probably value in consistency, at least to start with.
Comment 7•6 years ago
|
||
(In reply to :Paolo Amadini from comment #4)
> Part of the work will be the definition of the release criteria. This will
> involve sign-off from UX and engineering to ride the trains and replace
> "about:config". We aim to do this as part of the project timeline, but we
> realize that the release criteria is not defined by us alone, and we may
> need additional time to address feedback.
Please also take into account l10n. If you're placing the file in a "localizable" path, you're going to expose this experiment to all languages, and that's not OK if it's not ready for localization (or maybe not even going to ship).
Reporter | ||
Comment 8•6 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #7)
> Please also take into account l10n. If you're placing the file in a
> "localizable" path, you're going to expose this experiment to all languages,
> and that's not OK if it's not ready for localization (or maybe not even
> going to ship).
Yes, in fact this is something that I wanted your opinion on. On one hand, I agree we should avoid exposing strings to localizers that we may not have in the final product. On the other hand, if we start by hard-coding English strings, it may be more difficult to add localization as an after-thought.
I see now that the Web Payments project, that has a similar Nightly-only setup, solved this by defining the entities in the XHTML file itself, while avoiding hard-coding the strings in the markup:
https://searchfox.org/mozilla-central/source/browser/components/payments/res/paymentRequest.xhtml
Is there a similar setup that we can apply with Fluent? Maybe referencing an ".ftl" file that is not in the "locale" folder, so it's not exposed to localizers by our tools? In the code we used as reference, it seems the Fluent paths are all relative to the "locale" folder.
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(francesco.lodolo)
Comment 9•6 years ago
|
||
Redirecting to Axel, hoping he can answer that (note that he's traveling next week, not sure when he'll be online).
Flags: needinfo?(francesco.lodolo) → needinfo?(l10n)
Comment 10•6 years ago
|
||
(In reply to :Paolo Amadini from comment #5)
> (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #3)
> > Is there any reason not to use HTML directly instead of XHTML?
>
> We're using XHTML for most of out in-content pages, and also for the new
> "browser.xhtml".
>
> https://searchfox.org/mozilla-central/search?q=.xhtml
Most of those pages use xhtml to use dtd entities. XHTML docs have a lot of (sometimes unknown) special behavior compared to HTML and it would be a shame to accidentally start relying on them. The DevTools inspector is currently stuck using XHTML. Even though nothing obvious relies on XHTML in the inspector
(No dtd, no XUL element, ...), renaming the file to HTML causes behavior changes and test failures that are still unsolved.
It’d be nice we didn’t end up in the same situation here.
Also, HTML pages have proven to work well for Activity Stream and DevTools, so I don’t see why we can’t extend this here.
Comment 11•6 years ago
|
||
Need to play a little forward-the-forward here, this is mostly l10n registry, and then some build goop.
Zibi, what's your take on using Fluent for a not-yet-exposed-to-localizers UI? Another FileSource outside of /localization, and put en-US files in there? How does that scale/perform?
Flags: needinfo?(l10n) → needinfo?(gandalf)
Comment 12•6 years ago
|
||
(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #10)
> XHTML docs have a lot of (sometimes unknown) special behavior compared to HTML
Actually, it’s the other way around.
## Case in point:
- backwards incompatible void tag behaviour (attempting to use new self closing tags in browsers which don’t recognise them will create an incorrect DOM tree)
- <p> tag auto-closing (breaks nested <p>s)
- parser enforced <tbody> in a <table>
- <!DOCTYPE html> being needed to enable standards mode
- parser enforced presence of <html>, <body> and <head> tags (this is problematic for https://github.com/w3c/webcomponents/issues/752)
- `Element.tagName` is always upper case (this breaks some JavaScript frameworks when used in an XHTML environment, for example: https://github.com/FezVrasta/popper.js/pull/671)
- no support for self‑closing tags outside of SVG, MathML and void elements (needs to be supported by the parser)
XHTML doesn’t have the above problems as the XML file describes the DOM with no weird backwards incompatible edge cases.
Comment 13•6 years ago
|
||
> XHTML doesn’t have the above problems as the XML file describes the DOM with no weird backwards incompatible edge cases.
XHTML is pretty much an abandon format tho, while HTML is a web standard that everyone focuses on. While I believe that your list is accurate, I also expect it to be one sided, and I'd expect there to be a list of quirky behaviors of XHTML as well.
The main difference being is that writing for HTML means writing for the Web, while writing for XHTML means keeping ourselves in a Gecko-specific corner.
I'm saying that because my main work for the last 2 years is about writing DOM level software that has to support XUL, XHTML and HTML. I'd love to just support one of them.
I'll keep the NI on me for the question from Axel.
Reporter | ||
Comment 14•6 years ago
|
||
Thanks for adding some background. I discussed this briefly with Brian, and I think we will probably be fine with using HTML. If we find out that we need to import XUL elements, which hopefully we won't need, we can still create them using JavaScript, and they should work fine. Also, Brian is interested in fixing any bugs we may find in our support for HTML documents as privileged in-content pages.
So the first question here is, can we set up a valid alternative to the temporary XML entities that were used for localization in the Web Payments XHTML document?
Reporter | ||
Updated•6 years ago
|
Summary: Create a skeleton of the new internal settings XHTML page → Create a skeleton of the new internal settings HTML page
Updated•6 years ago
|
Attachment #9011241 -
Attachment description: Bug 1493446 - Create a skeleton of the new internal settings XHTML page. r?paolo → Bug 1493446 - Create a skeleton of the new internal settings HTML page. r?paolo,bgrins
Reporter | ||
Updated•6 years ago
|
Keywords: checkin-needed
QA Contact: enndeakin
Comment 15•6 years ago
|
||
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e994da668be6
Create a skeleton of the new internal settings HTML page. r=bgrins,paolo
Keywords: checkin-needed
Comment 16•6 years ago
|
||
Backout by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f52e3faa07a
Backed out changeset e994da668be6 for ES linting failure.
Comment 17•6 years ago
|
||
Backed out changeset e994da668be6 (Bug 1493446) for ES linting failure.
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=es&revision=e994da668be67893a6decb206427bb916bd30a25
Failure log - for ES: https://treeherder.mozilla.org/logviewer.html#?job_id=203625922&repo=autoland&lineNumber=259
- for build bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=203625922&repo=autoland&lineNumber=259
Backout link: https://hg.mozilla.org/integration/autoland/rev/6f52e3faa07ac606474f19de1422fc8fd305f33c
[task 2018-10-05T11:58:03.221Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2018-10-05T11:58:04.828Z] Installing setuptools, pip, wheel...done.
[task 2018-10-05T11:58:05.955Z] running build_ext
[task 2018-10-05T11:58:05.955Z] building 'psutil._psutil_linux' extension
[task 2018-10-05T11:58:05.955Z] creating build
[task 2018-10-05T11:58:05.955Z] creating build/temp.linux-x86_64-2.7
[task 2018-10-05T11:58:05.955Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-10-05T11:58:05.955Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-10-05T11:58:05.955Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-10-05T11:58:05.956Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-10-05T11:58:05.956Z] creating build/lib.linux-x86_64-2.7
[task 2018-10-05T11:58:05.956Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-10-05T11:58:05.956Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-10-05T11:58:05.956Z] building 'psutil._psutil_posix' extension
[task 2018-10-05T11:58:05.956Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-10-05T11:58:05.956Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-10-05T11:58:05.956Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-10-05T11:58:05.956Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-10-05T11:58:05.956Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-10-05T11:58:05.956Z]
[task 2018-10-05T11:58:05.956Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-10-05T12:02:59.714Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:4:1 | 'add_task' is not defined. (no-undef)
[task 2018-10-05T12:02:59.714Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:5:9 | 'BrowserTestUtils' is not defined. (no-undef)
[task 2018-10-05T12:02:59.715Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:6:5 | 'gBrowser' is not defined. (no-undef)
[task 2018-10-05T12:02:59.715Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:9:5 | 'info' is not defined. (no-undef)
[task 2018-10-05T12:02:59.715Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:10:12 | 'ContentTask' is not defined. (no-undef)
[task 2018-10-05T12:02:59.715Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:11:7 | 'Assert' is not defined. (no-undef)
[task 2018-10-05T12:02:59.715Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/components/aboutconfig/test/browser/browser_basic.js:11:20 | 'content' is not defined. (no-undef)
[taskcluster 2018-10-05 12:03:00.345Z] === Task Finished ===
[taskcluster 2018-10-05 12:03:00.345Z] Unsuccessful task run with exit code: 1 completed in 553.571 seconds
Flags: needinfo?(vincent.cote)
Reporter | ||
Comment 18•6 years ago
|
||
This just means that the ESLint configuration is missing in the folder with the tests. You can copy this file there:
https://searchfox.org/mozilla-central/source/browser/extensions/formautofill/test/browser/.eslintrc.js
To check whether this fixes the issue, you can use "./mach lint -wo".
Updated•6 years ago
|
QA Contact: enndeakin
Reporter | ||
Comment 19•6 years ago
|
||
There was another build failure mentioned in comment 17, that I didn't notice earlier, which is happening because the JS and CSS files we added were considered "duplicate". This is an automated test we have in place to ensure we keep the package size to a minimum. This isn't a real issue in this case, since this is built for Nightly only.
There is a whitelist of allowed duplicates, but for this case I just added some differing content to the files, since anyways we'll add real content in the next updates that will make the files unique again.
I'm running a tryserver build with this update and the ESLint fix:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=29e4250a5d8ea6a1438061efd8435a40c07fd50b
If this is green, I'll push the new version directly to mozilla-inbound.
Flags: needinfo?(vincent.cote)
Reporter | ||
Comment 20•6 years ago
|
||
New tryserver build with --no-artifact:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e75b861b80b1144cb29b039d5050f42b2d97457
Comment 21•6 years ago
|
||
> Zibi, what's your take on using Fluent for a not-yet-exposed-to-localizers UI? Another FileSource outside of /localization, and put en-US files in there? How does that scale/perform?
Why not just add them to the existing FileSource? We can try adding a new FileSource with just files for that, but due to the nature of L10nRegistry and the fact that we don't index files in it I'd expect there to be an I/O cost on startup because we need to check for each file's existence in the new FileSource.
We could make the new FileSource indexed (we have the IndexedFileSource class), which would remove that cost, or we could try to invest in adding `rkv` to L10nRegistry to cache the missing files per source so that only the first startup is affected, but I'm occupied with other projects and I won't have time for that.
Do you have a preference?
Flags: needinfo?(gandalf) → needinfo?(l10n)
Reporter | ||
Comment 22•6 years ago
|
||
(In reply to :Paolo Amadini from comment #20)
> New tryserver build with --no-artifact
The ESLint failure is because I forgot to put a newline at the end of the modified CSS and JS files. There's also a "browser_all_files_referenced.js" test failure that happens because we plan to only access the page via a direct "chrome:" URL, so nothing else is referencing this page at the moment. Here's another try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae7035f0d028f8b0f9b2416483bfd32f759bc87c
Comment 23•6 years ago
|
||
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bfaf2361851
Create a skeleton of the new internal settings HTML page. r=paolo,bgrins
Comment 24•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment 25•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #21)
> > Zibi, what's your take on using Fluent for a not-yet-exposed-to-localizers UI? Another FileSource outside of /localization, and put en-US files in there? How does that scale/perform?
>
> Why not just add them to the existing FileSource?
Good question. I was running on the assumption that that'd break repacks, but it might just not. Might be worth just trying, as long as we keep the ftl files out of l10n configs and dirs while we're playing around with 'em.
Flags: needinfo?(l10n)
Comment 26•6 years ago
|
||
If I had seen https://hg.mozilla.org/integration/mozilla-inbound/rev/0bfaf2361851#l1.13 before it landed, I would have requested the exception to be added in a way that enforces 'nightly-only' instead of just putting a comment saying "This is only in Nightly".
if (AppConstants.NIGHTLY_BUILD)
gExceptionPaths.push(...
would have been easy.
You need to log in
before you can comment on or make changes to this bug.
Description
•