1893345 - SelectTranslationsPanel footer buttons are in the wrong order on Windows

Status: VERIFIED FIXED

(Firefox :: Translations, defect, P3)

Description

[Erik Nordin [:nordzilla]]

Attachment: select-translations-panel-windows.png

Description

In Bug 1893100 D207602 I lowered the copy button into the footer in its own start-justified group. Likewise, I moved the other footer buttons into their own end-justified group.

This achieves the correct visual effect within the panel, but the Windows-specific button order is no longer correct (see screenshot).

We need to figure out what changes will allow the Windows-specific button-order logic to persist through this new structure of elements.

---

Before the regression

Prior to the regression the structure looked something like this:

<html:moz-button-group>
  <button></button>
  <button></button>
  <button default="true"></button>
  ...
</html:moz-button-group>

---

After the regression

After to the regression the structure looked something like this:

<html:moz-button-group>
  <html:div>
    <button></button>
  </html:div>
  <html:div>
    <button></button>
    <button></button>
    <button default="true"></button>
  </html:div>
</html:moz-button-group>

---

Itiel, do you have any ideas on how to fix this?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1870352

Comment 2

[Itiel]

I had this in mind during the review of earlier patches, and got to the conclusion this should be fine as-is.
AIUI, the order of the buttons is reversed only in the case of group of buttons related to one another, and only when that group is the only one in the footer.
Once you introduce another group (which consists of only 1 button in this case), the logic of reversing the order doesn't make sense to me- doing so would make the primary button in the middle of the buttons and not in the edge.
So either the bug should be wontfixed, or the copy button should move to be in the inline-end position (along with fixing this bug to make the Done button on the inline-start position), which needs to be approved by UX and it doesn't seem to be worth the hassle anyway.

At least that's how I see it...

Comment 3

[Dianna Smith [:diannaS]]

:nordzilla adding NI after Comment 2

Comment 4

[Erik Nordin [:nordzilla]]

Thanks Dianna!

I think I agree with Itiel's perspective here.

I am currently following up with UX and A11y to determine if this behavior is acceptable from their points of view as well.

---

Anna, I asked about this on Slack, but maybe it would be best to get your official perspective here in the bug comments.

Comment 5

[Anna Yeddi [:ayeddi]]

Erik, thank you for the ping and details!

Accessibility-wise it is important for the focus order to follow the visual order of elements. There are some minor exceptions (and two schools of thought how reasonable these exceptions are, I need to add), but this case is very straightforward:
If the focus order of the footer with Tab is Copy > Translate full page > Done (as it’s on the top screenshot, for the LTR language/English), It’s all good.

Consistency is important for navigation too, of course, but as Itiel has said above, the 3 button pattern is already differ from the regular 2 button one, thus it is more probable that the visual positioning of the primary/default button won't be as usual (or it's acceptable, rather). It's still styled differently which would aim sighted users in navigating this new pattern. Plus, it's unlikely there would be more than one panel opened at the same time, thus less confusion.

Comment 6

[Erik Nordin [:nordzilla]]

Attachment: Bug 1893345 - Fix SelectTranslationsPanel button order on Windows r=#translations-reviewers!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c964c84e678
Fix SelectTranslationsPanel button order on Windows r=translations-reviewers,desktop-theme-reviewers,gregtatum,Itiel

Comment 8

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in browser_translations_select_panel_translation_failure_after_unsupported_language.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/translations/tests/browser/browser_translations_select_panel_translation_failure_after_unsupported_language.js | The element 'select-translations-panel-translate-button' should have focus. - Got [object XULElement], expected [object XULElement]

Comment 9

[Erik Nordin [:nordzilla]]

Well that's frustrating.

I thought I had a clean try run for all of the operating systems on this patch stack:
https://treeherder.mozilla.org/jobs?repo=try&revision=fbd20e960ef800c7ce86f72f1c267ff94d25f4f9

Maybe I need to modify my query.

I'll look into fixing it!

Comment 10

[Erik Nordin [:nordzilla]]

Attachment: Bug 1893345 - Split SelectTranslationsPanel done button r=#translations-reviewers!

Splits the SelectTranslationsPanel done button into
two elements, one that is the primary button, and one
that is the secondary button, based on the context in
which it shows in the panel.

Depends on D209140

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/5c964c84e678

Comment 12

[Erik Nordin [:nordzilla]]

I do believe that my latest patch fixed the issue. Here is a clean try:

https://treeherder.mozilla.org/jobs?repo=try&revision=72a3378386a2c035bf0121420d0657d3a2eea700

---

:Serbans I think that you may have meant to change this to REOPENED instead of RESOLVED, since it was backed out.

I'm making that change since it seems like the right thing to do, and I'm queuing for re-landing, but I'm sending you a NI just in case.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27c2aded7b4b
Fix SelectTranslationsPanel button order on Windows r=translations-reviewers,desktop-theme-reviewers,gregtatum,Itiel
https://hg.mozilla.org/integration/autoland/rev/50ea04e087c7
Split SelectTranslationsPanel done button r=translations-reviewers,gregtatum

Comment 14

[Serban Stanca [:SerbanS]]

Yes, it's true. Sorry, my bad!

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/27c2aded7b4b
https://hg.mozilla.org/mozilla-central/rev/50ea04e087c7

Comment 16

[Timea Zsoldos [:tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 127.0a1 (2024.04.24) on Windows 10.
I can confirm this issue is fixed, I verified using Firefox 128.0a1 and Firefox 127.0b9 (flipping pref browser.translations.select.enable = true) on Windows 10, the buttons order is now correct for Windows.