Last Comment Bug 848291 - Update TransitionEvent to be compatible with the spec
: Update TransitionEvent to be compatible with the spec
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86_64 Linux
-- normal (vote)
: mozilla23
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
:
: Jet Villegas (:jet)
Mentors:
Depends on: 847599
Blocks: 868751
  Show dependency treegraph
 
Reported: 2013-03-06 03:56 PST by Olli Pettay [:smaug] (pto-ish for couple of days)
Modified: 2013-08-28 05:35 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (17.16 KB, patch)
2013-05-03 12:43 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
dbaron: review+
Details | Diff | Splinter Review
removed the comment in test (17.83 KB, patch)
2013-05-04 07:40 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
no flags Details | Diff | Splinter Review
additional patch for the test (871 bytes, patch)
2013-05-04 08:23 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
no flags Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-03-06 03:56:14 PST
Better to do this after we have webidl bindings
Comment 1 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-03 12:43:14 PDT
Created attachment 745299 [details] [diff] [review]
patch

This seems to work.
Note, the default values for the dictionary aren't in the spec, but by adding
default values, dictionary handling becomes a bit simpler in C++.

https://tbpl.mozilla.org/?tree=Try&rev=c355dd741e27
Comment 2 User image David Baron :dbaron: ⌚️UTC-8 2013-05-03 14:18:55 PDT
(In reply to Olli Pettay [:smaug] from comment #1)
> Note, the default values for the dictionary aren't in the spec, but by adding

Should they be?
Comment 3 User image David Baron :dbaron: ⌚️UTC-8 2013-05-03 14:28:14 PDT
Comment on attachment 745299 [details] [diff] [review]
patch

In test_transitions_events.html, you should fix:

>// We should get no events from transitions on pseudo-elements.
>$("seven").setAttribute("foo", "bar");

in two ways:

 (1) fix the comment
 (2) add two started_test() calls before it, since it is now expected to yield two events.  Otherwise the test will return before the last 2 tests finish.



I also think we should just remove initTransitionEvent, though at this point that should go in a separate patch.  Can you do that?
Comment 4 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-04 07:40:43 PDT
Created attachment 745550 [details] [diff] [review]
removed the comment in test
Comment 5 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-04 08:21:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1fac3f4bd83

but now I realized that perhaps I misunderstood the comment "in two ways"
Comment 6 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-04 08:23:57 PDT
Created attachment 745554 [details] [diff] [review]
additional patch for the test
Comment 7 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-04 08:25:50 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca8ab638f99
Comment 8 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2013-05-04 10:16:56 PDT
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #2)
> (In reply to Olli Pettay [:smaug] from comment #1)
> > Note, the default values for the dictionary aren't in the spec, but by adding
> 
> Should they be?
IMO it would make the spec easier to read.
Comment 9 User image David Baron :dbaron: ⌚️UTC-8 2013-05-04 10:43:22 PDT
(In reply to Olli Pettay [:smaug] from comment #8)
> (In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from
> comment #2)
> > (In reply to Olli Pettay [:smaug] from comment #1)
> > > Note, the default values for the dictionary aren't in the spec, but by adding
> > 
> > Should they be?
> IMO it would make the spec easier to read.

Does it change what the spec means?  It's not clear to me what's supposed to happen if a dictionary member without a default value is omitted.
Comment 10 User image David Baron :dbaron: ⌚️UTC-8 2013-05-04 10:47:02 PDT
I suppose if there's no default value, the spec is supposed to define what happens when it's not present.  (I don't see this happening much, though.)
Comment 11 User image David Baron :dbaron: ⌚️UTC-8 2013-05-04 10:49:43 PDT
Though it's not clear to me if giving the default values says anything about what happens when the entire dict is omitted.
Comment 13 User image David Baron :dbaron: ⌚️UTC-8 2013-05-06 17:24:57 PDT
spec fixed in https://dvcs.w3.org/hg/csswg/rev/172f4f12f059
Comment 14 User image David Baron :dbaron: ⌚️UTC-8 2013-05-06 17:33:28 PDT
... and https://dvcs.w3.org/hg/csswg/rev/3ee3ec0b5a5b

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