Follow up to bug 1304350 We want to update naming scheme based on :pike's decision and refactor the Localization class slightly according to Stas' proposal.
Summary: Apply renaming + refactor of Localization/DOMLocalization → Refactor Localization into DOMLocalization
Created attachment 8810562 [details] [review] Pull request Here's a WIP. I need to fix comments and docstrings, but wanted to get your early feedback on the general direction. Let me know if you have any questions!
Assignee: gandalf → stas
Status: NEW → ASSIGNED
Attachment #8810562 - Flags: feedback?(gandalf)
Comment on attachment 8810562 [details] [review] Pull request Yeah, if you get good perf results I'm ok with this landing before release.
Attachment #8810562 - Flags: feedback?(gandalf) → feedback+
I updated the PR. All tests now pass. Here's a summary of changes. - The old LocalizationObserver class is no more. Instead, there are two subclasses of Localization: DOMLocalization and DocumentLocalization. - DOMLocalization is a subclass of Localization and implements methods for translating DOM trees and elements, like translateFragment and translateElement. Localization itself is environment-agnostic. Right now it still sanitizes the arguments in a way that is specific to HTML, but I'll fix that in bug 1315975 or a follow-up. - DocumentLocalization is a subclass of DOMLocalization. In HTML/XUL documents, an instance of DocumentLocalization will be available as document.l10n. It implements the observer interface and supports delegating to sub-localizations. The sub-localizations are stored in the document.l10n.delegates map. - I considered factoring the observer interface into its own class or a mixin, but eventually decided to leave it as part of DocumentLocalization to keep things simple. - We used to have two methods related to observing roots: observeRoot and disconnectRoot. They would serve two purposes: 1) observe/unobserve the root via the MutationObserver instance and 2) add/remove the root to/from the list of known roots used by top-down methods like translateRoots. In this PR, I separated these two purposes into two pairs of methods: - observeRoot and unobserveRoot are available only on DocumentLocalization (because it implements the observer interface) and they only handle 1) from above; they aren't part of the public API, - connectRoot and disconnectRoot are available on every DOMLocalization (because now every DOMLocalization including DocumentLocalization has its own set of roots) and it handles 2) from above; it will also use observeRoot/unobserveRoot if they're available; thus, connectRoot and disconnectRoot can be considered part of the public API.
Comment on attachment 8810562 [details] [review] Pull request Zibi, this is now ready for your review. See comment 6 for a summary of changes.
Attachment #8810562 - Flags: review?(gandalf)
Unfortunately the windows and macos builds failed. The linux builds show no perf impact of this change: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=3dd4ca4c35eb&newProject=try&newRevision=919b65e98ca7379dfa7aca2685fbf9bb12d818c2&framework=1&showOnlyImportant=0
Comment on attachment 8810562 [details] [review] Pull request awesome work :stas! r+ :)
Attachment #8810562 - Flags: review?(gandalf) → review+
https://hg.mozilla.org/projects/larch/rev/4de23d5b8758f314a92b3409087d6ba01ddb64dc Bug 1316083 - Refactor Localization into DOMLocalization. r=gandalf
Thanks for the review!
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.