[Webkit-unassigned] [Bug 60184] Introduce HTML5 track list objects

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60184





--- Comment #6 from Eric Carlson <eric.carlson at apple.com>  2011-05-19 08:00:12 PST ---
(From update of attachment 93773)
View in context: https://bugs.webkit.org/attachment.cgi?id=93773&action=review

> Source/WebCore/dom/ExclusiveTrackList.cpp:76
> +void ExclusiveTrackList::select(long index, ExceptionCode& ec)
> +{
> +    if (index != NoSelection && !checkIndex(index, ec))
> +        return;
> +
> +    // Don't assert since it can be null in degenerate cases like frames detached from their pages.
> +    if (!scriptExecutionContext())
> +        return;
> +
> +    ASSERT(scriptExecutionContext()->isContextThread());
> +    scriptExecutionContext()->postTask(DispatchTask<ExclusiveTrackList, long>::create(this, &ExclusiveTrackList::onSelect, index));
> +}
> +
> +void ExclusiveTrackList::onSelect(long index)
> +{
> +    ASSERT(index >= NoSelection && index < static_cast<long>(length()));
> +    m_selectedIndex = index;
> +
> +    dispatchEvent(Event::create(eventNames().changeEvent, false, false));
> +}

Shouldn't the track be selected synchronously (in select) and only the 'change' event posted asynchronously? The spec only says that the event should happen in the task:

  The select(index) must select the track with index index,
  if there is one, unselecting whichever track was previously
  selected. If there is no track with index index, it must 
  instead throw an INDEX_SIZE_ERR exception.

  Whenever the selected track is changed, the user agent 
  must queue a task to fire a simple event named change 
  at the MultipleTrackList object.

http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#dom-tracklist-select

Also, is there any reason to not use dispatchChangeEvent() in the base class?



> Source/WebCore/dom/ExclusiveTrackList.idl:32
> +        // FIXME: the spec says unsigned long, but -1 is used when nothing is selected.
> +        // A bug has been already submitted to the spec draft.

It would be good to include the bug url here.


> Source/WebCore/dom/MultipleTrackList.cpp:81
> +void MultipleTrackList::onEnable(unsigned long index)
> +{
> +    ASSERT(index < length());
> +    m_isEnabled[index] = true;
> +
> +    dispatchEvent(Event::create(eventNames().changeEvent, false, false));
> +}

Ditto the question about select() above.


> Source/WebCore/dom/MultipleTrackList.cpp:102
> +void MultipleTrackList::onDisable(unsigned long index)
> +{
> +    ASSERT(index < length());
> +    m_isEnabled[index] = false;
> +
> +    dispatchEvent(Event::create(eventNames().changeEvent, false, false));
> +}

And here.


> Source/WebCore/dom/TrackList.cpp:136
> +ScriptExecutionContext* TrackList::scriptExecutionContext() const
> +{
> +    // FIXME: provide an script execution context for HTML Media Element and MediaStream API use cases.
> +    return 0;
> +}

Bug number?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.