Keyboard settings can't be pre-localized when handwriting layout included

RESOLVED FIXED in 2.2 S1 (5dec)

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: timdream, Assigned: wdeng)

Tracking

({regression})

unspecified
2.2 S1 (5dec)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 fixed)

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
stas
: review+
Details | Review
STR:

1. Remove in-HTML English labels in apps/keyboard/settings.html
2. |GAIA_KEYBOARD_LAYOUTS=en,zh-Hans-Handwriting APP=keyboard make install-gaia|
3. Settings -> Keyboard -> Built-in Keyboard

Expected:

1. See labels

Actual:

1. Empty screen

Note: 

mozL10n emit the following error when the id clearly exists in keyboard.en-US.properties

[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: settingsPageTitle
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: vibration
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: clickSound
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: autoCorrect
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: wordSuggestion
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: handwritingKeyboard
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: strokeWidth
]
[/build_stage/keyboard/settings.html: mozL10n: A non-existing entity requested: responseSpeed

I am not sure who is at fault here since this bug only happens when we run the step annotating settings.html. We probably need to someone to trace the build step here.

PS: We are indeed missing some ids for the handwriting section; I plan to add them in bug 1098124 if this is not fixed in time.
Flags: needinfo?(wdeng)
Flags: needinfo?(stas)
Flags: needinfo?(gandalf)
Assignee

Comment 1

5 years ago
Yes, missing some label values for l10-ids in handwriting settings section, but I don't think it's the root cause of "empty screen".

Tried to reproduce it step by step as above, I can see the error log, but also I can see the labels, even switch system language to Traditional Chinese(from English), I can still see the labels though they are English for handwriting part.

My gaia commit: a6c295a7a6bddf5bcd42d970725300c4c30760b8

I will update to the latest code and try it again.
Flags: needinfo?(wdeng)
It looks like the problem is with the utils.writeContent function which escapes the braces in the URI template for the l10n resource.

Before the following line is run

  https://github.com/mozilla-b2g/gaia/blob/a710590a62889f0a9b3484e0b22ab4d3abf638c5/apps/keyboard/build/settings-config.js#L56

the href of the localization link is

  locales/keyboard.{locale}.properties

Right after it's run, it's:

  locales/keyboard.%7Blocale%7D.properties

Which means that the pretranslation mechanism in

  https://github.com/mozilla-b2g/gaia/blob/a710590a62889f0a9b3484e0b22ab4d3abf638c5/build/webapp-optimize.js#L152

can't fetch the proper file and fails.
Flags: needinfo?(stas)
Actually, I might be wrong.  It looks like the file is fetch correctly, even if the braces are escaped.
Do you have bandwidth to work on this bug?
Flags: needinfo?(gandalf) → needinfo?(wdeng)
Assignee

Updated

5 years ago
Assignee: nobody → wdeng
Flags: needinfo?(wdeng)
Assignee

Comment 5

5 years ago
Yes, I will try it.
Assignee

Comment 6

5 years ago
It's ridiculous, serialize DOM tree with nsIDOMSerializer, see [1], it seems some dom element attributes, like href='url', string values are encoded with encodeURI.

[1]https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIDOMSerializer
Attachment #8525954 - Flags: review?(timdream)
Wei, would you mind moving this logic into apps/keyboard/build/settings-config.js instead?  Maybe just before the following line is run:

   utils.writeContent(utils.getFile(distDirPath, 'settings.html'), sDoc);

I don't think it belongs in l10n.js.
Comment on attachment 8525954 [details] [review]
patch - pull request 26324

According :stas, this doesn't seem to be the reason why localize failed?
Attachment #8525954 - Flags: review?(timdream) → review?(stas)
Assignee

Comment 9

5 years ago
Hi stas,
   Thank you for your advice. Agree, for only this bug, I think we can do it the way you suggested.

   But I think, modifying source files in building is general for many apps, the building process diagram, see[1], localization is in the final phase(post-app.js). It means, l10n.js would translate all the changed html files, no matter how they were modified. So, I think make changes in l10n.js can make it more robust.

Now, the settings.html is changed as: settings.html->string->DOM(manipulate DOM)->string->settings.html.
It's too complex, actually I dislike it very much. A more elegent way I think is:
  1, put all the content in one single file settings.html,
  2, comment some parts(js in <head> and other sections in <body> ),
  3. in the building process, we can uncomment any comment.

But, I still don't know how to uncomment comments, do you have any ideas?

This patch can be suspended.

Thank you.

[1] https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Build_System_Primer#Build_process
Comment on attachment 8525954 [details] [review]
patch - pull request 26324

Never mind, :stas give his comment 3 hours ago.
Attachment #8525954 - Flags: review?(stas)
Assignee

Comment 11

5 years ago
Comment on attachment 8525954 [details] [review]
patch - pull request 26324

hi stas:
  Do you have time take a look? Thank you.
Attachment #8525954 - Flags: review?(stas)
Comment on attachment 8525954 [details] [review]
patch - pull request 26324

r=me, thanks!
Attachment #8525954 - Flags: review?(stas) → review+
Assignee

Updated

5 years ago
Master: https://github.com/mozilla-b2g/gaia/commit/68040c891b20e05abcb4c9e7bbb6e893baf66607
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S1 (5dec)
plus'ing fixed regression.
blocking-b2g: 2.2? → 2.2+
Keywords: regression
You need to log in before you can comment on or make changes to this bug.