Closed Bug 1886843 Opened 4 months ago Closed 3 months ago

Component-based screenshot UI swaps the order of "Save Visible" & "Save Full Page" buttons (maybe by accident?)

Categories

(Firefox :: Screenshots, defect, P3)

defect

Tracking

()

VERIFIED FIXED
126 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox124 --- disabled
firefox125 --- disabled
firefox126 --- verified

People

(Reporter: dholbert, Assigned: niklas)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image image.png

STR:

  1. Right-click a page and choose "Take Screenshot"
  2. Look at the buttons in the top right corner, to save visible vs. full-page. Pay attention to which one is first.
  3. Now toggle the about:config pref screenshots.browser.component.enabled to change its value.
  4. Repeat steps 1-2.

ACTUAL RESULTS:
The buttons are in a different order depending on the value of that pref:

  • With screenshots.browser.component.enabled = true, the button on the left is "Save Visible".
  • With screenshots.browser.component.enabled = false, the button on the left is "Save Full Page.
    Nightly has the pref defaulting to true, so it shows the former ordering. Release has the pref defaulting to false, so it shows the latter

EXPECTED RESULTS:
They probably should be in the same order that they were before, to benefit from muscle-memory & so people who use this feature don't get spooked by stuff unnecessarily moving? (Unless we're making a conscious choice to reorder them as part of this rewrite, if the new order is definitively better for some reason. If that's the case, feel free to close this.)

See attached screenshot, with the true pref on top and the false pref on bottom.

Blocks: 1870127
Keywords: regression
Regressed by: 1789727
See Also: → 1883765
Summary: Component-based screenshot UI swaps the order of "Save Visible" & "Save Full Page" → Component-based screenshot UI swaps the order of "Save Visible" & "Save Full Page" buttons (maybe by accident?)

:niklas, since you are the author of the regressor, bug 1789727, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(nbaumgardner)

Thanks for spotting this. I think this change was probably accidental, but looking at our telemetry and thinking about it now, I think it makes sense for the "full page" button to be the primary button in that button group (full page is used ~30% more). This means it would get styled differently, receive initial focus and - if we can manage it - follow the same rules for appearing on the left vs. right in windows vs. macos as our other button groups.

This will mean a mismatch with the extension, but an intentional one.

Flags: needinfo?(nbaumgardner)
Severity: -- → S3
Priority: -- → P3
Assignee: nobody → nbaumgardner
Status: NEW → ASSIGNED
Attachment #9392620 - Attachment description: Bug 1886843 - Use moz-button-group for screenshots-buttons. r=sfoster → WIP: Bug 1886843 - Use moz-button-group for screenshots-buttons. r=sfoster
Attachment #9392620 - Attachment description: WIP: Bug 1886843 - Use moz-button-group for screenshots-buttons. r=sfoster → Bug 1886843 - Use moz-button-group for screenshots-buttons. r=sfoster
Pushed by nbaumgardner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a69aca2a396
Use moz-button-group for screenshots-buttons. r=sfoster,desktop-theme-reviewers
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch

While verifying this using the latest Nightly 127.0a1, I noticed a variation in button arrangement across different operating systems, as follows:

  • macOS 13 and Ubuntu 22.04: "Save visible" is positioned before "Save full page" from left to right.
  • Windows 10: "Save full page" appears before "Save visible" from left to right.

Sam or Niklas, is this difference in arrangement intentional?

Flags: needinfo?(sfoster)
Flags: needinfo?(nbaumgardner)

Yes, that is intentional. We made the save full page button the primary button and the primary button is on the right for mac and linux and on the left for windows.

Flags: needinfo?(sfoster)
Flags: needinfo?(nbaumgardner)

(In reply to Niklas Baumgardner [:niklas] from comment #7)

Yes, that is intentional. We made the save full page button the primary button and the primary button is on the right for mac and linux and on the left for windows.

Yeah this follows OS conventions and should be consistent with all the other dialogs and toolbars in Firefox UI, across each platform.

I can confirm this is fixed also on Firefox 126.0b5 when the component version of Screenshots is enabled (screenshots.browser.component.enabled is set to true) - tested on Ubuntu 22.04, macOS 13 and Windows 10.

Based on this and on Comments 6, 7 and 8 - setting this to Verified fixed.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: