Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1967493002:
Serializing <keygen> and <track> should not generate end tags. (Closed)
|
Created:
4 years, 7 months ago by ramya.v Modified:
4 years, 7 months ago CC:
blink-reviews, blink-reviews-html_chromium.org, blink-reviews-w3ctests_chromium.org, chromium-reviews, dglazkov+blink, tfarina, bengr, mdjones, nyquist Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSerializing <keygen> and <track> should not generate end tags.
Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments
Merged along with
Roll DOM Distiller JavaScript distribution package
Diff since last roll: https://github.com/chromium/dom-distiller/compare/2b5538e628...0adf24afe4
Picked up changes: 0adf24a <track> should not have end tags
BUG=521771
Patch Set 1 #Patch Set 2 : Updated test #
Messages
Total messages: 24 (8 generated)
Description was changed from ========== [WIP] BUG= ========== to ========== Serializing <keygen> and <track> should not generate end tags. Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments BUG=521771 ==========
ramya.v@samsung.com changed reviewers: + tkent@chromium.org
PTAL! Thanks
The CQ bit was checked by tkent@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1967493002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1967493002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2016/05/11 05:51:38, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) Its failing in DomDistillerJsTest.RunJsTests (track element generating end tag) https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium.... Expected=<a href="http://example.com/link"></a><img src="http://example.com/image" srcset="http://example.com/image200 200w, http://example.com/image400 400w"><img src="http://example.com/image2"><video src="http://example.com/video" poster="http://example.com/poster"><source src="http://example.com/source"><track src="http://example.com/track"></track></video> Actual =<a href="http://example.com/link"></a><img src="http://example.com/image" srcset="http://example.com/image200 200w, http://example.com/image400 400w"><img src="http://example.com/image2"><video src="http://example.com/video" poster="http://example.com/poster"><source src="http://example.com/source"><track src="http://example.com/track"></video> Is it possible to change the expectations? The tests are done in org.chromium.distiller package but I could not find the individual java files (DOMUtilTest.java, WebVideoTest.java etc) to modify the expectations.
Adding dom_distiller owners.
On 2016/05/11 06:45:42, tkent wrote: > Adding dom_distiller owners. The code is at https://github.com/chromium/dom-distiller We'll fix the expectation and prepare a rolling CL like https://codereview.chromium.org/1917033004/ Then you can combine them to land atomically.
On 2016/05/11 07:10:33, wychen wrote: > On 2016/05/11 06:45:42, tkent wrote: > > Adding dom_distiller owners. > > The code is at https://github.com/chromium/dom-distiller > We'll fix the expectation and prepare a rolling CL like > https://codereview.chromium.org/1917033004/ > Then you can combine them to land atomically. The fix is here: https://codereview.chromium.org/1971443005/
On 2016/05/11 21:19:16, wychen wrote: > On 2016/05/11 07:10:33, wychen wrote: > > On 2016/05/11 06:45:42, tkent wrote: > > > Adding dom_distiller owners. > > > > The code is at https://github.com/chromium/dom-distiller > > We'll fix the expectation and prepare a rolling CL like > > https://codereview.chromium.org/1917033004/ > > Then you can combine them to land atomically. > > The fix is here: > https://codereview.chromium.org/1971443005/ The rolling CL is here: https://codereview.chromium.org/1971023002/ Could you merge that one here, and also copy the whole CL description and append it to yours? Thanks!
wychen@chromium.org changed reviewers: + wychen@chromium.org
Description was changed from ========== Serializing <keygen> and <track> should not generate end tags. Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments BUG=521771 ========== to ========== Serializing <keygen> and <track> should not generate end tags. Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments Merged along with Roll DOM Distiller JavaScript distribution package Diff since last roll: https://github.com/chromium/dom-distiller/compare/2b5538e628...0adf24afe4 Picked up changes: 0adf24a <track> should not have end tags BUG=521771 ==========
Description was changed from ========== Serializing <keygen> and <track> should not generate end tags. Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments Merged along with Roll DOM Distiller JavaScript distribution package Diff since last roll: https://github.com/chromium/dom-distiller/compare/2b5538e628...0adf24afe4 Picked up changes: 0adf24a <track> should not have end tags BUG=521771 ========== to ========== Serializing <keygen> and <track> should not generate end tags. Spec: https://html.spec.whatwg.org/multipage/syntax.html#serialising-html-fragments Merged along with Roll DOM Distiller JavaScript distribution package Diff since last roll: https://github.com/chromium/dom-distiller/compare/2b5538e628...0adf24afe4 Picked up changes: 0adf24a <track> should not have end tags BUG=521771 ==========
On 2016/05/12 00:42:53, wychen wrote: > On 2016/05/11 21:19:16, wychen wrote: > > On 2016/05/11 07:10:33, wychen wrote: > > > On 2016/05/11 06:45:42, tkent wrote: > > > > Adding dom_distiller owners. > > > > > > The code is at https://github.com/chromium/dom-distiller > > > We'll fix the expectation and prepare a rolling CL like > > > https://codereview.chromium.org/1917033004/ > > > Then you can combine them to land atomically. > > > > The fix is here: > > https://codereview.chromium.org/1971443005/ > > The rolling CL is here: > https://codereview.chromium.org/1971023002/ > > Could you merge that one here, and also copy the whole CL description and append > it to yours? Thanks! Merged the CL and description. Could you please take a look? Thanks
On 2016/05/12 04:55:47, ramya.v wrote: > Merged the CL and description. Could you please take a look? > > Thanks The merge doesn't seem right. You can: git checkout <your branch for this CL> git reset --hard <sha1 corresponding to patch set 2> git cl patch 1971023002 git cl upload The the diff between patch set 4 and 2 should be exactly patch 1971023002.
On 2016/05/12 05:35:56, wychen wrote: > On 2016/05/12 04:55:47, ramya.v wrote: > > Merged the CL and description. Could you please take a look? > > > > Thanks > > The merge doesn't seem right. > You can: > git checkout <your branch for this CL> > git reset --hard <sha1 corresponding to patch set 2> > git cl patch 1971023002 Oops. You might need git cl issue 1967493002 to set the associated issue number back, before uploading. > git cl upload > > The the diff between patch set 4 and 2 should be exactly patch 1971023002.
Patchset #3 (id:40001) has been deleted
On 2016/05/12 05:39:07, wychen wrote: > On 2016/05/12 05:35:56, wychen wrote: > > On 2016/05/12 04:55:47, ramya.v wrote: > > > Merged the CL and description. Could you please take a look? > > > > > > Thanks > > > > The merge doesn't seem right. > > You can: > > git checkout <your branch for this CL> > > git reset --hard <sha1 corresponding to patch set 2> > > git cl patch 1971023002 > Oops. > You might need > git cl issue 1967493002 > to set the associated issue number back, before uploading. > > > git cl upload > > > > The the diff between patch set 4 and 2 should be exactly patch 1971023002. How to get sha1 corresponding to patch set2? (which is not committed)? Just FYI I've done the following in previous cl. (which I deleted now) Checked out to a new branch (git pull --rebase and gclient sync) git cl patch 1967493002 on top of this commit, added patch 1971023002 manually. (Downloaded raw patch and applied patch -p1 < abc.patch) git add -u and git commit --amend git cl upload (During this step I got this presubmit error File is stale: build/android/test_runner.pydeps. To regenerate run: build/print_python_deps.py --root build/android --output build/android/test_runner.pydeps build/android/test_runner.py If I run that command additional changes are coming in the commit. Without that I'm unable to submit.
On 2016/05/12 06:21:19, ramya.v wrote: > How to get sha1 corresponding to patch set2? (which is not committed)? I meant your local sha1. > Just FYI > I've done the following in previous cl. (which I deleted now) > Checked out to a new branch (git pull --rebase and gclient sync) > git cl patch 1967493002 > on top of this commit, added patch 1971023002 manually. (Downloaded raw patch > and applied patch -p1 < abc.patch) > git add -u and git commit --amend > git cl upload (During this step I got this presubmit error > File is stale: build/android/test_runner.pydeps. To regenerate run: > build/print_python_deps.py --root build/android --output > build/android/test_runner.pydeps build/android/test_runner.py > If I run that command additional changes are coming in the commit. Without that > I'm unable to submit. I just tried what I said, and here's the CL: https://codereview.chromium.org/1971963003/ It didn't complain about the file, and running the command didn't change it.
On 2016/05/12 06:40:43, wychen wrote: > On 2016/05/12 06:21:19, ramya.v wrote: > > How to get sha1 corresponding to patch set2? (which is not committed)? > I meant your local sha1. > > > Just FYI > > I've done the following in previous cl. (which I deleted now) > > Checked out to a new branch (git pull --rebase and gclient sync) > > git cl patch 1967493002 > > on top of this commit, added patch 1971023002 manually. (Downloaded raw patch > > and applied patch -p1 < abc.patch) > > git add -u and git commit --amend > > git cl upload (During this step I got this presubmit error > > File is stale: build/android/test_runner.pydeps. To regenerate run: > > build/print_python_deps.py --root build/android --output > > build/android/test_runner.pydeps build/android/test_runner.py > > If I run that command additional changes are coming in the commit. Without > that > > I'm unable to submit. > > I just tried what I said, and here's the CL: > https://codereview.chromium.org/1971963003/ > > It didn't complain about the file, and running the command didn't change it. Will check why its giving this error on my machine. We can proceed with the new cl for now I feel. Can you add tkent as reviewer to the https://codereview.chromium.org/1971963003/. Will close this patch.
On 2016/05/12 08:53:28, ramya.v wrote: > Will check why its giving this error on my machine. We can proceed with the new > cl for now I feel. > Can you add tkent as reviewer to the > https://codereview.chromium.org/1971963003/. > Will close this patch. Landed. Feel free to close this one. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
