Update TransitionEvent to be compatible with the spec

RESOLVED FIXED in mozilla23

Status

()

Core
Layout
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

({dev-doc-complete})

unspecified
mozilla23
x86_64
Linux
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

4 years ago
Better to do this after we have webidl bindings
(Assignee)

Updated

4 years ago
Assignee: nobody → bugs
(Assignee)

Comment 1

4 years ago
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
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+
(Assignee)

Comment 4

4 years ago
Created attachment 745550 [details] [diff] [review]
removed the comment in test
(Assignee)

Updated

4 years ago
Blocks: 868751
(Assignee)

Comment 5

4 years ago
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
(Assignee)

Updated

4 years ago
Blocks: 868751
(Assignee)

Comment 6

4 years ago
Created attachment 745554 [details] [diff] [review]
additional patch for the test
(Assignee)

Comment 7

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca8ab638f99
(Assignee)

Comment 8

4 years ago
(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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
spec fixed in https://dvcs.w3.org/hg/csswg/rev/172f4f12f059
... and https://dvcs.w3.org/hg/csswg/rev/3ee3ec0b5a5b
I documented this:
https://developer.mozilla.org/en-US/docs/Web/API/TransitionEvent
https://developer.mozilla.org/en-US/docs/Web/API/TransitionEvent.pseudoElement
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/23
Keywords: dev-doc-complete
You need to log in before you can comment on or make changes to this bug.