Closed Bug 748315 Opened 13 years ago Closed 8 years ago

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

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla58

Tracking Flags:

Tracking

Status

firefox58

---

fixed

People

(Reporter: ayg, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Bug 748315 - Part 1. Consider text-align property even if HTMLEditor::IsCSSEnabled() is false. 8 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 748315 - Part 2. SetAttributeOrEquivalent should remove CSS property when HTMLEditor::IsCSSEnabled() is false. 8 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 748315 - Part 3. Use SetAttributeOrEquivalent even if HTMLEditor::IsCSSEnabled() is false. 8 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 748315 - Part 4. Update web-platform-tests result. 8 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 748315 - Part 5. Update browserscope test result. 8 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details

Aryeh Gregor (:ayg) (no longer with Mozilla)

Reporter

Description

•

13 years ago

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+

Makoto Kato [:m_kato]

Assignee

Updated

•

8 years ago

Assignee: nobody → m_kato

Makoto Kato [:m_kato]

Assignee

Comment 1

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d54c7e6e94d4272809b06c7dbcad368cea955243

Makoto Kato [:m_kato]

Assignee

Comment 2

•

8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c9ee40bc1061d65c0a421761fa4b6a50225c573

Comment hidden (mozreview-request)

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)

Comment 8

•

8 years ago

mozreview-review

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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)

Comment 9

•

8 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)

Comment 10

•

8 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)

Comment 11

•

8 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)

Comment 12

•

8 years ago

mozreview-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+

Comment hidden (mozreview-request)

Pulsebot

Comment 18

•

8 years ago

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

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 19

•

8 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/68e52184e901 https://hg.mozilla.org/mozilla-central/rev/f5d83a9ca1b0 https://hg.mozilla.org/mozilla-central/rev/5904a51460e2 https://hg.mozilla.org/mozilla-central/rev/73804363647e https://hg.mozilla.org/mozilla-central/rev/2b51a6cf8f4c

Status: NEW → RESOLVED

Closed: 8 years ago

status-firefox58: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla58

You need to log in before you can comment on or make changes to this bug.