Closed Bug 848291 Opened 7 years ago Closed 6 years ago

Update TransitionEvent to be compatible with the spec

Categories

(Core :: Layout, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: smaug, Assigned: smaug)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

Better to do this after we have webidl bindings
Assignee: nobody → bugs
Attached patch patchSplinter Review
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
Attachment #745299 - Flags: review?(dbaron)
(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 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?
Attachment #745299 - Flags: review?(dbaron) → review+
Blocks: 868751
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1fac3f4bd83

but now I realized that perhaps I misunderstood the comment "in two ways"
No longer blocks: 868751
Blocks: 868751
(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.
(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.
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.)
Though it's not clear to me if giving the default values says anything about what happens when the entire dict is omitted.
https://hg.mozilla.org/mozilla-central/rev/e1fac3f4bd83
https://hg.mozilla.org/mozilla-central/rev/0ca8ab638f99
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.