Set lang attribute to the translated language after translation
Categories
(Firefox :: Translations, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox123 | --- | fixed |
People
(Reporter: marco, Assigned: wartmanm)
Details
Attachments
(1 file)
It looks like Google Translate and Safari Translate do this as well, according to https://github.com/mozilla/firefox-translations/issues/727.
|
||
Comment 1•2 years ago
|
||
This is probably an accessibility thing as well, although I don't think macOS VoiceOver supports multilingual voices. It's quite comical testing VoiceOver on Spanish pages with an English voice.
Following this over from Github. As I mentioned over there, I'd be happy to implement this. I have some experience contributing to Firefox already, albeit on my personal account.
|
|
||
Comment 3•2 years ago
|
||
That would be great! Let me know if you need any pointers.
The translations document is where this will need to happen:
I think the existing tests will just need to be updated, and no new ones created:
Care will need to be taken for what happens with bare text nodes, as a parent element may need to be modified. I haven't thought through this completely though. It might be good to test other implementations.
|
|
||
Comment 4•2 years ago
|
||
We also have lots of Outreachy applicants picking up translations bugs this month, so we're only assigning bugs right now when there is a patch up.
I hadn't really considered how arbitrary nodes might need to be updated, that makes things trickier.
I did some more testing, and Safari and Chrome modify the root <html lang="..."> attribute if one is present, but don't add one. They don't seem to update lang attributes on other elements, or to pay any attention to them for translation purposes. Edge doesn't modify lang attributes at all.
Since Firefox does honor lang attributes, would it be appropriate to update them (so long as they match the source language) but follow suit and not add new ones?
|
|
||
Updated•2 years ago
|
|
|
||
Comment 6•2 years ago
|
||
Let's make it simple. I would say we should only modify the <html> lang attribute once we do a full page translation into that language. I can verify that Safari and Chrome modify the root HTML. If we get more complicated with translating subroots of the page or selection translation in place, we can revisit this then.
|
||
Updated•2 years ago
|
I used the existing viewportTranslated promise rather than adding a new one to wait for the full page translation, as it seemed like it was in the same spirit and I didn't want to make it more complex. I can add a fullPageTranslated promise if desired, though.
That being said, viewportTranslated seems to be firing too early, when the title node is translated rather than body. Should I file a separate bug to address that?
|
|
||
Comment 9•2 years ago
|
||
That being said, viewportTranslated seems to be firing too early, when the title node is translated rather than body. Should I file a separate bug to address that?
Yeah it would be good to get that fixed if that's the case! We're only using it for telemetry purposes at this moment.
|
||
Comment 10•2 years ago
|
||
There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:wartmanm, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.
|
|
||
Comment 11•2 years ago
|
||
I plan on landing this on Monday after the soft freeze.
|
|
||
Updated•2 years ago
|
|
|
||
Comment 12•2 years ago
|
||
So I tried to land it before the break, and unfortunately it needs rebasing. I'm not sure how I can do that myself. Sorry for the really long process here, but would you mind seeing if you can do that :mattheww?
|
||
Comment 13•2 years ago
|
||
Sorry for disappearing! It's rebased and should be ready to go.
|
|
||
Comment 14•2 years ago
|
||
This has been a bit of "action at a distance", but I hit land! Let's hope it sticks. Thanks for the contribution.
|
||
Comment 15•2 years ago
|
||
|
||
Comment 16•2 years ago
|
||
| bugherder | ||
|
|
||
Updated•2 years ago
|
Description
•