Closed Bug 1532703 Opened 6 years ago Closed 6 years ago

Allow HTML-based about:config to ride the trains to release once Product has signed off

Categories

(Toolkit :: Preferences, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
relnote-firefox --- 71+
firefox70 --- wontfix
firefox71 + fixed

People

(Reporter: mconley, Assigned: Paolo)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

This bug will track the work to allow about:config to ride up to Beta and Release once Product has signed off on what's in Nightly.

This is effectively the reverse of bug 1529088.

See Also: → 1529088
Priority: -- → P3
Blocks: 1524782
Type: defect → enhancement

Mark Striemer has just landed the last bug of the UX refresh of "about:config", and the completed refresh will be ready to be tested in Nightly by tomorrow. That was the last blocker from the additional MVP scope that we defined earlier this year, and I believe we're now on track to release the new "about:config" in Firefox 71.

We have about one week before soft code freeze, and if no new elements emerge, it should be more than enough time.

.

This should be an easy decision. There is significant product risk every time we don't let the new "about:config" ride the trains, because we don't catch regressions in the old "about:config" until Beta, as the old version is not covered by regression tests. With us moving to shorter release cycles, the risk of catching regressions very late in the Beta stage or missing them altogether only increases.

At the same time, the new page has now been tested in Nightly for six months and no new blockers have been found. It has extensive regression tests, and I've made sure to review the recent code changes to keep this test coverage and prevent new regressions from slipping in. Mark and I are discussing some enhancements in bug 1582535, but they are not blocking and I'm still making sure that, while making these improvements, we don't accidentally regress other aspects of the design.

.

To summarize, the main change since earlier in the year is the use of icons instead of text for the action buttons. With this, we've reintroduced double click as an interaction to flip or edit a preference. This compensates for the fact that text was slightly clearer than icons, and icons are a smaller click target. I've seen several sources already mentioning double click to flip a preference, and the buttons still exist in icon format as the discoverable way of using the main feature of the page.

.

I'm setting needinfos for the next steps as I understand them:

  • Markus to confirm that what we've correctly implemented all the blockers from the UX refresh;
  • Mark to ensure that we didn't miss anything on the implementation side or behind the scenes;
  • Andreas to review the feature overall and provide the Product sign off;
  • In my role as feature champion, it is my understanding that it is now ready for release.
Flags: needinfo?(mstriemer)
Flags: needinfo?(mjaritz)
Flags: needinfo?(abovens)

(In reply to :Paolo Amadini from comment #1)

  • Markus to confirm that what we've correctly implemented all the blockers from the UX refresh;

Looks good to me.
I expect we can improve the UI for adding an entry by separating it better as it still blends with the table and therefor might be hard to find in some cases, but this is not a blocker.

Flags: needinfo?(mjaritz)

Thanks Markus! I filed bug 1586918 to keep track of the improvement.

Will this also be used on mobile?

Flags: needinfo?(abovens) → needinfo?(paolo.mozmail)

This isn't being used Firefox Preview or Fennec.

I think the implementation is ready to go now from my end.

Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(mstriemer)

[Tracking Requested - why for this release]: We want to get this shipped in 71 so we can remove the old about:config. Waiting on final sign off.

Changing the priority to p2 as the bug is tracked by a release manager for the current nightly.
See What Do You Triage for more information

Priority: P3 → P2

(In reply to Mark Striemer [:mstriemer] from comment #6)

Waiting on final sign off.

Thanks for looking into this, setting the needinfo for Andreas.

Mark, the patch for this bug should probably combine a backout of bug 1529088 (except for browser_label_textlink.js) and of bug 1532001. I'd provide a patch, but getting non-artifact builds working on my system is nowhere near completion at the moment...

Flags: needinfo?(abovens)

Also, as per the original plan we should keep the old page at chrome://global/content/config.xul for general mitigation for at least one release, even though we don't need to address minor regressions that may occur there as part of the de-XUL process. Bug 1524782 tracks its removal.

A few things from my side:

  • Double clicking a pref no longer works to toggle / edit: why is that?
  • The critter image is very large: not a blocker perhaps, but can we make it smaller?
  • Has the new UI passed a11y review? I want to make sure Asa & team have seen it.

Also, I'd like Chuck to chime in from a desktop PM perspective.

Flags: needinfo?(charmston)
Flags: needinfo?(asa)
Flags: needinfo?(abovens)

(In reply to Andreas Bovens [:abovens] from comment #11)

  • Double clicking a pref no longer works to toggle / edit: why is that?

Have you updated to the most recent Nightly before testing? Double click should toggle a preference value.

Flags: needinfo?(abovens)

(In reply to Andreas Bovens [:abovens] from comment #11)

  • The critter image is very large: not a blocker perhaps, but can we make it smaller?

The critter image has also been replaced. It should look like this now (see attachment).

(In reply to Andreas Bovens [:abovens] from comment #11)

  • Has the new UI passed a11y review? I want to make sure Asa & team have seen it.

We also haven't changed the accessibility design since the previous version that was already signed off.

Also, comment 1 in this bug outlines in more detail the reasons why we're tracking this for Firefox 71.

We've recently revived the project and finally made the UX changes that were requested since the original version. This was the only part missing from the version that was almost ready to ship in Firefox 67.

(In reply to :Paolo Amadini from comment #12)

(In reply to Andreas Bovens [:abovens] from comment #11)

  • Double clicking a pref no longer works to toggle / edit: why is that?

Have you updated to the most recent Nightly before testing? Double click should toggle a preference value.

You have to click on the button, but clicking the full line no longer works.

(In reply to Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos) from comment #13)

Created attachment 9100508 [details]
Screen Shot 2019-10-11 at 1.36.00 PM.png

(In reply to Andreas Bovens [:abovens] from comment #11)

  • The critter image is very large: not a blocker perhaps, but can we make it smaller?

The critter image has also been replaced. It should look like this now (see attachment).

I updated, and indeed, I get this new image. It's still fairly large though (not a blocker, to be clear).

Flags: needinfo?(abovens)

(In reply to Andreas Bovens [:abovens] from comment #16)

You have to click on the button, but clicking the full line no longer works.

Do you mean double click the full line? Single click on the button, or double click on the rest of the line, should toggle a Boolean preference or start editing a String or Number (by design). We have regression tests specifically for this and it works in my local build, even in the corners of the row, so if you have the right build I don't see what could be missing.

Maybe check the Build ID in "about:support" to see the date when your Nightly was built?

Flags: needinfo?(abovens)

Double-clicking an individual line works for me now as well. Odd it didn't work earlier.

So, as far as I am concerned, this is good to ride the trains :)

Flags: needinfo?(asa)
Flags: needinfo?(abovens)

Chuck will be sharing his notes here momentarily as well.

A few notes from my testing:

  • The "show all" button in the initial screen doesn't look like a button and it is not immediately clear that it is actionable. I'd suggest that it look more like the "Accept the risk and continue" button on the previous screen.
  • I'm not confident that the interactive elements (e.g. a <span> that turns into a <form><input></form> on click) are sufficiently annotated with ARIA attributes. Labels are helpful, but dynamic form elements are particularly tricky to get right, and I don't have immediate access to a screen reader to test myself.

I agree that we should get further accessibility signoff, and I'd like the UX team to look at this as well (in the process of finding someone to help with this). I don't expect any major issues to be raised, and I understand that there's pressure to move this to beta in the next day or two, so I'd propose:

  • Move this to beta this week (ni?Liz to make sure this plan works with release management).
  • Hold it in beta until we have signoff from Asa and UX.
  • Depending on the severity of any issues found in those signoffs, either let it continue to ride the trains and follow up with fixes in the next cycle, or hold it one more release to let the fixes catch up.

Does this all sound reasonable?

Flags: needinfo?(lhenry)
Flags: needinfo?(charmston)
Flags: needinfo?(asa)

(In reply to Chuck Harmston [:chuck] from comment #21)

  • The "show all" button in the initial screen doesn't look like a button and it is not immediately clear that it is actionable. I'd suggest that it look more like the "Accept the risk and continue" button on the previous screen.

This is an intentional UX design choice. We styled this using what the design guide calls a "ghost button" because Telemetry has shown that almost all uses of the current "about:config" start with a search.

A prominent "Show All" button would make the flow unclear, drawing attention away from the "Search" box which is the main element.

I agree that we should get further accessibility signoff, and I'd like the UX team to look at this as well (in the process of finding someone to help with this).

On the UX side, the latest refresh was led by Markus Jaritz, who already signed off in comment 2 on this bug. If there are more questions from Product he may be able to speak to those, but I'm inclined to think that by now there are no issues that make this page unshippable to our users.

  • Move this to beta this week (ni?Liz to make sure this plan works with release management).
  • Hold it in beta until we have signoff from Asa and UX.
  • Depending on the severity of any issues found in those signoffs, either let it continue to ride the trains and follow up with fixes in the next cycle, or hold it one more release to let the fixes catch up.

Sounds good with one small correction. Due to the setup of tests on this feature, we can't hold this on Beta and not ship on Release, but we can let it move on to early Beta and then back it out of Beta if necessary - even before Beta 1 is built. The latter eventuality, in my opinion, should be very unlikely to happen anyways.

It looks like you intend this for beta 71, so definitely talk with Pascal who is the 71 release owner.

Flags: needinfo?(lhenry) → needinfo?(pascalc)

Thank you all for the great progress we made today! It's nice to see that this feature is finally completing, and also the continuous push from everyone to get it to improve in the best way we can. In fact, while we had already gotten accessibility addressed and signed off, in recent days we had already started discussing other non-blocking improvements to it. We're always thinking ahead ;-)

I'm really confident that this will go smoothly with no major blockers, and now that we've gotten two Product Managers to review the landing plan, I can go ahead and push the patch now so it doesn't bitrot - the feature is already tracked for Firefox 71 anyways.

I ran an up-to-date regular tryserver build and a Beta simulation:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=89c425a97be032ed49b4bf752eddce883a46559b&selectedJob=271147814
https://treeherder.mozilla.org/#/jobs?repo=try&revision=86a3a7d785ff1914be0e1e7e39f0ddb65fc97337&selectedJob=271150012

Both look good to me, with known perma-failures looking like bug 1587447 and bug 1588295. In case there are issues I didn't notice, landing now will give us more time to handle any regression in the time leading to Beta, making life easier for Release Managers as well - and if there are concerns I'm free in the following days to address them.

(In reply to Chuck Harmston [:chuck] from comment #21)

  • Hold it in beta until we have signoff from Asa and UX.

The UX of it good to ship.

The show all button is intentional as Paolo explained.
We have a bug on file to improve the add row. (Bug 1586918)
The image size is a good catch. I filed bug 1588563
(non of those are a blocker however)

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/autoland/rev/1008047de53e Redirect "about:config" to the HTML version on all channels for browser builds. r=mconley
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → paolo.mozmail

The enabling patch has built successfully, and as the last step it seems we just need some clarity on accessibility.

If there are any newly identified blockers (issues that prevent users of accessibility technology from using the page or make it very difficult, thus making it unsuitable for shipping to Release), we need to know by Friday because in that case we might have to do the work to disable the feature in early beta on Monday, before the Beta 3 go-to-build.

I don't expect blockers because we have involved the accessibility team from the beginning, during the development of the feature. By now, it has almost certainly been used routinely in Nightly by users of accessibility technology. I have also already reached out to Asa Dotzler, James Teh, and Marco Zehe on Tuesday to solicit comments.

It may also be useful to know which improvements would be high priority and if there are additional recommendations, even though these may be reported at any time. We have some ideas already in the latter category, such as bug 1560683 for moving up and down with arrow keys, in addition to the standard tabbing which already works.

Chuck mentioned we might improve the dynamic form controls, so if there is input on how exactly to do that, it may turn out to be an easy, upliftable patch. I don't think it would be blocking anyways, because at the moment, even if the field to edit a value is not specially tagged, it is focused right after activating a button named "Edit", contains the current value of the preference, and is followed by a primary button named "Save", and all inside a table row whose accessibility header is the preference name.

Compare this with the old "about:config", where the fact that you can edit preferences is not advertised at all, as far as I can tell. Unless you have used the page before, you have to discover that the edit commands are located in the context menu, or try to activate the row.

This means to me that new HTML-based version is probably an overall accessibility improvement for the most common use case already, because all the commands are in the natural page flow and labelled, following web pages conventions.

To close the loop on this, Asa has been out of the office this week but replied to me saying that while he noticed some minor quirks, he thinks overall the feature seems solid and it's reasonable to let it ride the trains.

Flags: needinfo?(pascalc)
Flags: needinfo?(asa)

Is this something that we may want to add to release notes for 71? If so can you suggest wording for a note? Thanks!

Flags: needinfo?(paolo.mozmail)

This change is less visible than, say, the Downloads Panel redesign in Firefox 54, but is probably more noticeable for users who regularly consult release notes. Thus, we could add a release note in the "changed" category, or even "developer", if the latter is appropriate for changes that are not specifically in DevTools code. I suggest this wording:

Redesigned internal configuration page (about:config)

This is for Firefox Desktop only.

relnote-firefox: --- → ?
Flags: needinfo?(paolo.mozmail)

(In reply to :Paolo Amadini from comment #31)

This change is less visible than, say, the Downloads Panel redesign in Firefox 54, but is probably more noticeable for users who regularly consult release notes. Thus, we could add a release note in the "changed" category, or even "developer", if the latter is appropriate for changes that are not specifically in DevTools code. I suggest this wording:

Redesigned internal configuration page (about:config)

Added to our upcoming 71 beta release notes, thanks.

Regressions: 1502867
No longer regressions: 1502867
Regressions: 1601899
Regressions: 1604240
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: