Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1487)

Issue 98793007: Revert 160318 "Remove TreatNullAs=NullString for media interfaces" (Closed)

Created:
7 years ago by haraken
Modified:
6 years, 11 months ago
Reviewers:
philipj_slow
CC:
blink-reviews, nessy, arv+blink, feature-media-reviews_chromium.org, dglazkov+blink, Inactive, vcarbune.chromium, adamk+blink_chromium.org
Visibility:
Public.

Description

Revert 160318 "Remove TreatNullAs=NullString for media interfaces" > Remove TreatNullAs=NullString for media interfaces > > The spec doesn't have special null handling here: > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html > > The updated test was run in Firefox Nightly (with media.webvtt.enabled), > IE11 Release Preview and Opera 12.16 (Presto 2.12.388). Everything > passes, with these exceptions: > > No other browser supports the mediaGroup property. > > The preload default value is "metadata" in IE11 and Presto, and "" in > Firefox. The spec suggests "metadata". Bugs filed: > https://code.google.com/p/chromium/issues/detail?id=310450 > https://bugzilla.mozilla.org/show_bug.cgi?id=929890 > > BUG=310298 > > Review URL: https://codereview.chromium.org/35803004 TBR=philipj@opera.com Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164124

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -30 lines) Patch
M LayoutTests/fast/dom/element-attribute-js-null.html View 7 chunks +9 lines, -18 lines 0 comments Download
M LayoutTests/fast/dom/element-attribute-js-null-expected.txt View 3 chunks +7 lines, -7 lines 0 comments Download
M Source/core/html/HTMLMediaElement.idl View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/html/HTMLSourceElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLTrackElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLVideoElement.idl View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
haraken
7 years ago (2013-12-18 23:40:20 UTC) #1
haraken
Committed patchset #1 manually as r164124.
7 years ago (2013-12-18 23:40:34 UTC) #2
philipj_slow
On 2013/12/18 23:40:34, haraken wrote: > Committed patchset #1 manually as r164124. Why did this ...
7 years ago (2013-12-19 09:09:31 UTC) #3
haraken
On 2013/12/19 09:09:31, philipj wrote: > On 2013/12/18 23:40:34, haraken wrote: > > Committed patchset ...
7 years ago (2013-12-19 10:18:24 UTC) #4
acolwell GONE FROM CHROMIUM
On 2013/12/19 09:09:31, philipj wrote: > On 2013/12/18 23:40:34, haraken wrote: > > Committed patchset ...
7 years ago (2013-12-19 14:18:51 UTC) #5
philipj_slow
On 2013/12/19 14:18:51, acolwell wrote: > On 2013/12/19 09:09:31, philipj wrote: > > On 2013/12/18 ...
6 years, 11 months ago (2014-01-13 11:07:37 UTC) #6
acolwell GONE FROM CHROMIUM
On 2014/01/13 11:07:37, philipj wrote: > On 2013/12/19 14:18:51, acolwell wrote: > > On 2013/12/19 ...
6 years, 11 months ago (2014-01-13 18:00:30 UTC) #7
philipj_slow
On 2014/01/13 18:00:30, acolwell wrote: > On 2014/01/13 11:07:37, philipj wrote: > > On 2013/12/19 ...
6 years, 11 months ago (2014-01-14 02:14:46 UTC) #8
acolwell GONE FROM CHROMIUM
On 2014/01/14 02:14:46, philipj wrote: > On 2014/01/13 18:00:30, acolwell wrote: > > On 2014/01/13 ...
6 years, 11 months ago (2014-01-14 02:34:44 UTC) #9
philipj_slow
6 years, 11 months ago (2014-01-14 09:59:16 UTC) #10
Message was sent while issue was closed.
On 2014/01/14 02:34:44, acolwell wrote:
> On 2014/01/14 02:14:46, philipj wrote:
> > On 2014/01/13 18:00:30, acolwell wrote:
> > > On 2014/01/13 11:07:37, philipj wrote:
> > > > On 2013/12/19 14:18:51, acolwell wrote:
> > > > > On 2013/12/19 09:09:31, philipj wrote:
> > > > > > On 2013/12/18 23:40:34, haraken wrote:
> > > > > > > Committed patchset #1 manually as r164124.
> > > > > > 
> > > > > > Why did this need to be reverted?
> > > > > 
> > > > > It exposed a bug in layout when the poster URL is invalid and a width
is
> > > > > specified. I'll be uploading a fix shortly.
> > > > 
> > > > Is there an open bug or code review for this now?
> > > 
> > > I landed a fix last week.
> > > https://src.chromium.org/viewvc/blink?revision=164643&view=revision
> > 
> > So, I'm surprised to find that this revert commit doesn't actually seem to
be
> on
> > master, there is no SVN r164124 to be found. That means I don't have to
revert
> > the revert, but how did this actually happen? Was this committed on some
other
> > branch?
> 
> This was only reverted on the stable branch. I opted to keep your change on
beta
> & ToT and fix the regression properly instead of just restoring the old
> behavior.

Great, thanks! I suppose I could have noticed that by the
"branches/chromium/1700/" prefix in
https://src.chromium.org/viewvc/blink?view=rev&revision=164124

Powered by Google App Engine
This is Rietveld 408576698