Last Comment Bug 738440 - Support queryCommandState("stylewithcss")
: Support queryCommandState("stylewithcss")
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla14
Assigned To: Aryeh Gregor (:ayg) (next working March 28-April 26)
:
: Makoto Kato [:m_kato]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 13:37 PDT by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2012-04-04 04:46 PDT (History)
2 users (show)
ayg: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.74 KB, patch)
2012-03-22 14:27 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
ehsan: review+
Details | Diff | Splinter Review
Patch v2, test failure fixed (6.06 KB, patch)
2012-03-23 12:06 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
ehsan: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-03-22 13:37:15 PDT
WebKit supports it, and it makes sense, so I specced it.  The spec requires non-support of state for useCSS.
Comment 1 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-03-22 14:27:19 PDT
Created attachment 608478 [details] [diff] [review]
Patch v1

I think the old behavior was just a bug.  The code set STATE_ATTRIBUTE to a boolean, but it's supposed to be a string.  So when QueryCommandValue() tried retrieving "state_attribute" as a string, it found nothing.  And when QueryCommandState() tried retrieving "state_all" as a boolean, it also found nothing.  All this just demonstrates that the code here is sufficiently confusing that it baffled the people who wrote it . . .

I had to add a special case for useCSS, because the spec says queryCommandState() shouldn't be supported for it.  It's legacy cruft, and I want to support as few features as possible for it.  Plus, we'd need a special case for it anyway, because we'd have to invert the return value if it were to make any sense.
Comment 2 Mozilla RelEng Bot 2012-03-22 14:31:06 PDT
Autoland Patchset:
	Patches: 608478
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=4c9d8f3ff72d
Try run started, revision 4c9d8f3ff72d. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=4c9d8f3ff72d
Comment 3 :Ehsan Akhgari 2012-03-22 16:11:21 PDT
Comment on attachment 608478 [details] [diff] [review]
Patch v1

Review of attachment 608478 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, but I think you should also modify http://mxr.mozilla.org/comm-central/source/mozilla/content/html/content/test/test_bug408231.html.
Comment 4 Mozilla RelEng Bot 2012-03-23 01:47:02 PDT
Try run for 4c9d8f3ff72d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4c9d8f3ff72d
Results (out of 219 total builds):
    exception: 2
    success: 171
    warnings: 32
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-4c9d8f3ff72d
Comment 5 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-03-23 12:06:16 PDT
Created attachment 608809 [details] [diff] [review]
Patch v2, test failure fixed

Same as last patch, but with test_bug408231.html fixed.  I wrote the test based on bug 738366 being fixed, so it should be false rather than true.  Thus bug 738366 needs to land first, or at the same time.  This patch will conflict with the bug 738385 changes, but that's okay, it should be easy to merge.

(I don't know why queryCommandValue("stylewithcss") is suddenly returning the empty string instead of throwing, but it's moot, because it will do that anyway after bug 738385 lands.)
Comment 6 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-04-03 07:16:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfcdf9a80f36
Comment 7 Marco Bonardo [::mak] 2012-04-04 04:46:08 PDT
https://hg.mozilla.org/mozilla-central/rev/cfcdf9a80f36

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