Closed Bug 748315 Opened 11 years ago Closed 5 years ago

queryCommandState("justify*") should respect CSS text-align values

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: ayg, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Test-case:

data:text/html,<!DOCTYPE html>
<div contenteditable><div style="text-align:center">foo</div></div>
<script>
getSelection().collapse(document.querySelector("div").firstChild.firstChild, 1);
document.body.textContent = document.queryCommandState("justifycenter");
</script>

We output false, WebKit outputs true.  The spec requires WebKit's behavior.  Tested by richtext2:

RTE2-QS_JC_SPANs:ta:c-1_SI
RTE2-QS_JC_SPAN.jc-1-SI
RTE2-QS_JC_MYJC-1-SI

and similarly for the other justify*.  (I'm not sure if the latter two tests are actually correct, though.)
Flags: in-testsuite+
Assignee: nobody → m_kato
Comment on attachment 8922692 [details]
Bug 748315 - Part 1. Consider text-align property even if HTMLEditor::IsCSSEnabled() is false.

https://reviewboard.mozilla.org/r/193842/#review198930
Attachment #8922692 - Flags: review?(masayuki) → review+
Comment on attachment 8922693 [details]
Bug 748315 - Part 2. SetAttributeOrEquivalent should remove CSS property when HTMLEditor::IsCSSEnabled() is false.

https://reviewboard.mozilla.org/r/193844/#review198934
Attachment #8922693 - Flags: review?(masayuki) → review+
Comment on attachment 8922694 [details]
Bug 748315 - Part 3. Use SetAttributeOrEquivalent even if HTMLEditor::IsCSSEnabled() is false.

https://reviewboard.mozilla.org/r/193846/#review198936

Thank you for creating a series of small patches. That really make me review easier!!

::: editor/libeditor/HTMLEditRules.cpp:8736
(Diff revision 1)
> -    if (HTMLEditUtils::SupportsAlignAttr(aElement)) {
> -      rv = htmlEditor->SetAttribute(&aElement, nsGkAtoms::align, aAlignType);
> +  if (!HTMLEditUtils::SupportsAlignAttr(aElement)) {
> +    // XXX error?

Then, please use NS_WARN_IF here and check the trysever's raw log.
Attachment #8922694 - Flags: review?(masayuki) → review+
Comment on attachment 8922695 [details]
Bug 748315 - Part 4. Update web-platform-tests result.

https://reviewboard.mozilla.org/r/193848/#review198938

::: commit-message-851e4:1
(Diff revision 1)
> +Bug 748315 - Part 4. Updat web-platform-tests result. r?masayuki

s/Updat/Update
Attachment #8922695 - Flags: review?(masayuki) → review+
Comment on attachment 8922696 [details]
Bug 748315 - Part 5. Update browserscope test result.

https://reviewboard.mozilla.org/r/193850/#review198940
Attachment #8922696 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/68e52184e901
Part 1. Consider text-align property even if HTMLEditor::IsCSSEnabled() is false. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/f5d83a9ca1b0
Part 2. SetAttributeOrEquivalent should remove CSS property when HTMLEditor::IsCSSEnabled() is false.  r=masayuki
https://hg.mozilla.org/integration/autoland/rev/5904a51460e2
Part 3. Use SetAttributeOrEquivalent even if HTMLEditor::IsCSSEnabled() is false. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/73804363647e
Part 4. Update web-platform-tests result. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/2b51a6cf8f4c
Part 5. Update browserscope test result. r=masayuki
You need to log in before you can comment on or make changes to this bug.