1062602 - gaia-subheader should have semantic structure for text

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Components, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Currently, gaia-subheader use looks like this:

<gaia-subheader>
  Foo
</gaia-subheader>

internally, the components adds a scoped style element:

<gaia-subheader>
  Foo
  <style scoped />
</gaia-subheader>

That messes up with l10n because when you set data-l10n-id on gaia-subheader element, you basically ask it to localize the content of the element, including the <style/> element.

The solution may be one of the two:

1) Add a span:

<gaia-subheader>
  <span>Foo</span>
</gaia-subheader>

That's how all other components work

2) Move the content and potentially data-l10n-id to span inside script.js

Sth like this:

   proto.createdCallback = function() {
+    var span = document.createElement('span');
+    if (this.hasAttribute('data-l10n-id')) {
+      span.setAttribute('data-l10n-id', this.getAttribute('data-l10n-id'));
+      this.removeAttribute('data-l10n-id');
+    }
+    span.textContent = this.textContent;
+    this.textContent = '';
+    this.appendChild(span);
     ComponentUtils.style.call(this, baseurl);
   };

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Wilson, can you give me feedback on the proposed course of action?

It blocks us from moving forward with bug 1053629.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: pull request

Some apps already use <span/> (like system), but others (and component's example) does not.

This patch just adds it everywhere.

Comment 3

[Wilson Page [:wilsonpage]]

Comment on attachment 8484082 [details] [review]
pull request

I understand the problem.

The span solution works and I think we should probably land it.

Perhaps a better solution would be to override the default .textContent and .innerHTML with custom setters. To make sure the <style> node is not erased.

Although when Gecko shadow-dom implementation is fully complete we won't need this pesky <style> tag and this issue will go way.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Overriding .textContent sounds like a good solution, but I'm not sure how it will work with DOM mutations provided from the Parser.

Not having the pesky <style/> sounds even better :)

Commit: https://github.com/mozilla-b2g/gaia/commit/638a6f6f414511e2a86cedae2bc1c24bc8aabddc
Merge: https://github.com/mozilla-b2g/gaia/commit/7deecddd430884d5eed2266b7b4f3bb0a8ff661c

Thanks!

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

And a small follow-up to that patch: fix to ringtones test 

https://github.com/mozilla-b2g/gaia/commit/8eafcbbf43bf0e8d4b1335356e446c36da77de6e