34302 2010-01-28 20:25:32 -0800 RenderVideo should prefer provided width/height properties to hard coded defaults when natural size is unavailable 2010-01-28 23:42:17 -0800 1 1 1 Unclassified WebKit Media 528+ (Nightly build) All All RESOLVED FIXED P2 Enhancement --- 0 nicholas.young webkit-unassigned commit-queue eric.carlson oldest_to_newest 185707 0 nicholas.young 2010-01-28 20:25:32 -0800 The constructor of RenderVideo in WebCore/rendering/RenderVideo.cpp attempts to determine the intrinsic size of the video by calling the naturalSize() method of the media player. However, when RenderVideo is constructed the media player may not yet have been constructed. In these situations, the renderer currently falls back to a hard coded default video size as the intrinsic size - (300, 150). This patch contends that in these situations, the width and height properties of the video element, if available, should be used as the intrinsic size until better values are available. If the width and height properties are unavailable, then the hard coded default is used once again. 185710 1 47669 nicholas.young 2010-01-28 20:28:38 -0800 Created attachment 47669 Initial Patch. 185728 2 47669 eric.carlson 2010-01-28 21:42:47 -0800 Comment on attachment 47669 Initial Patch. > // size since they also have audio thrown at them. By setting the intrinsic > // size to 300x1 the video will resize itself in these cases, and audio will > // have the correct height (it needs to be > 0 for controls to render properly). > - if (video->ownerDocument() && video->ownerDocument()->isMediaDocument()) > + if (video->hasAttribute(widthAttr) && video->hasAttribute(heightAttr)) > + setIntrinsicSize(IntSize(video->width(), video->height())); > + else if (video->ownerDocument() && video->ownerDocument()->isMediaDocument()) This comment doesn't make sense now, it is about the second test. Please add a comment about the logic in the first test ("Prefer provided video element width/height properties...") and move this comment down to be with the second test. r=me with this change 185732 3 47678 nicholas.young 2010-01-28 21:52:03 -0800 Created attachment 47678 Updated Patch Comments reorganized and updated as requested. 185737 4 47678 eric.carlson 2010-01-28 22:14:26 -0800 Comment on attachment 47678 Updated Patch Nice change, thanks! r=me 185755 5 47678 commit-queue 2010-01-28 23:42:11 -0800 Comment on attachment 47678 Updated Patch Clearing flags on attachment: 47678 Committed r54048: <http://trac.webkit.org/changeset/54048> 185756 6 commit-queue 2010-01-28 23:42:17 -0800 All reviewed patches have been landed. Closing bug. 47669 2010-01-28 20:28:38 -0800 2010-01-28 21:52:03 -0800 Initial Patch. webkit-isize.txt text/plain 2535 nicholas.young SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NDA0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMTAtMDEtMjggIE5pY2hvbGFzIFlvdW5nICA8bmljaG9sYXMueW91 bmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFByZWZlciBwcm92aWRlZCB2aWRlbyBlbGVtZW50IHdpZHRoL2hlaWdodCBwcm9wZXJ0 aWVzIHRvIGhhcmQgY29kZWQKKyAgICAgICAgZGVmYXVsdHMgZm9yIGludHJpbnNpYyBzaXplIHdo ZW4gbmF0dXJhbCB2aWRlbyBzaXplIGlzIHVuYXZhaWxhYmxlLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzQzMDIKKworICAgICAgICBObyBuZXcgdGVz dHMgbmVlZGVkLgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclZpZGVvLmNwcDogQXR0ZW1w dCB0byB1c2Ugd2lkdGgvaGVpZ2h0IHByb3BlcnRpZXMKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRl clZpZGVvOjpSZW5kZXJWaWRlbyk6CisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclZpZGVvLmg6 IE1vcmUgYXBwcm9wcmlhdGUgY29uc3RydWN0b3Igc2lnbmF0dXJlCisKIDIwMTAtMDEtMjggIFN0 ZXZlIEJsb2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5 IERhdmlkIExldmluLgpJbmRleDogV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlkZW8uY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZGVvLmNwcAkocmV2aXNpb24g NTQwMzkpCisrKyBXZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWRlby5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTUwLDcgKzUwLDcgQEAgdXNpbmcgbmFtZXNwYWNlIEhUTUxOYW1lczsKIHN0YXRpYyBj b25zdCBpbnQgY0RlZmF1bHRXaWR0aCA9IDMwMDsKIHN0YXRpYyBjb25zdCBpbnQgY0RlZmF1bHRI ZWlnaHQgPSAxNTA7CiAKLVJlbmRlclZpZGVvOjpSZW5kZXJWaWRlbyhIVE1MTWVkaWFFbGVtZW50 KiB2aWRlbykKK1JlbmRlclZpZGVvOjpSZW5kZXJWaWRlbyhIVE1MVmlkZW9FbGVtZW50KiB2aWRl bykKICAgICA6IFJlbmRlck1lZGlhKHZpZGVvKQogewogICAgIGlmICh2aWRlby0+cGxheWVyKCkp CkBAIC02MCw3ICs2MCw5IEBAIFJlbmRlclZpZGVvOjpSZW5kZXJWaWRlbyhIVE1MTWVkaWFFbGVt ZW4KICAgICAgICAgLy8gc2l6ZSBzaW5jZSB0aGV5IGFsc28gaGF2ZSBhdWRpbyB0aHJvd24gYXQg dGhlbS4gQnkgc2V0dGluZyB0aGUgaW50cmluc2ljCiAgICAgICAgIC8vIHNpemUgdG8gMzAweDEg dGhlIHZpZGVvIHdpbGwgcmVzaXplIGl0c2VsZiBpbiB0aGVzZSBjYXNlcywgYW5kIGF1ZGlvIHdp bGwKICAgICAgICAgLy8gaGF2ZSB0aGUgY29ycmVjdCBoZWlnaHQgKGl0IG5lZWRzIHRvIGJlID4g MCBmb3IgY29udHJvbHMgdG8gcmVuZGVyIHByb3Blcmx5KS4KLSAgICAgICAgaWYgKHZpZGVvLT5v d25lckRvY3VtZW50KCkgJiYgdmlkZW8tPm93bmVyRG9jdW1lbnQoKS0+aXNNZWRpYURvY3VtZW50 KCkpCisgICAgICAgIGlmICh2aWRlby0+aGFzQXR0cmlidXRlKHdpZHRoQXR0cikgJiYgdmlkZW8t Pmhhc0F0dHJpYnV0ZShoZWlnaHRBdHRyKSkKKyAgICAgICAgICAgIHNldEludHJpbnNpY1NpemUo SW50U2l6ZSh2aWRlby0+d2lkdGgoKSwgdmlkZW8tPmhlaWdodCgpKSk7CisgICAgICAgIGVsc2Ug aWYgKHZpZGVvLT5vd25lckRvY3VtZW50KCkgJiYgdmlkZW8tPm93bmVyRG9jdW1lbnQoKS0+aXNN ZWRpYURvY3VtZW50KCkpCiAgICAgICAgICAgICBzZXRJbnRyaW5zaWNTaXplKEludFNpemUoY0Rl ZmF1bHRXaWR0aCwgMSkpOwogICAgICAgICBlbHNlCiAgICAgICAgICAgICBzZXRJbnRyaW5zaWNT aXplKEludFNpemUoY0RlZmF1bHRXaWR0aCwgY0RlZmF1bHRIZWlnaHQpKTsKSW5kZXg6IFdlYkNv cmUvcmVuZGVyaW5nL1JlbmRlclZpZGVvLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9yZW5kZXJp bmcvUmVuZGVyVmlkZW8uaAkocmV2aXNpb24gNTQwMzkpCisrKyBXZWJDb3JlL3JlbmRlcmluZy9S ZW5kZXJWaWRlby5oCSh3b3JraW5nIGNvcHkpCkBAIC00MCw3ICs0MCw3IEBAIGNsYXNzIEdyYXBo aWNzTGF5ZXI7CiAKIGNsYXNzIFJlbmRlclZpZGVvIDogcHVibGljIFJlbmRlck1lZGlhIHsKIHB1 YmxpYzoKLSAgICBSZW5kZXJWaWRlbyhIVE1MTWVkaWFFbGVtZW50Kik7CisgICAgUmVuZGVyVmlk ZW8oSFRNTFZpZGVvRWxlbWVudCopOwogICAgIHZpcnR1YWwgflJlbmRlclZpZGVvKCk7CiAKICAg ICB2b2lkIHZpZGVvU2l6ZUNoYW5nZWQoKTsK 47678 2010-01-28 21:52:03 -0800 2010-01-28 23:42:11 -0800 Updated Patch webkit-isize.txt text/plain 3321 nicholas.young SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NDA0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMTAtMDEtMjggIE5pY2hvbGFzIFlvdW5nICA8bmljaG9sYXMueW91 bmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFByZWZlciBwcm92aWRlZCB2aWRlbyBlbGVtZW50IHdpZHRoL2hlaWdodCBwcm9wZXJ0 aWVzIHRvIGhhcmQgY29kZWQKKyAgICAgICAgZGVmYXVsdHMgZm9yIGludHJpbnNpYyBzaXplIHdo ZW4gbmF0dXJhbCB2aWRlbyBzaXplIGlzIHVuYXZhaWxhYmxlLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzQzMDIKKworICAgICAgICBObyBuZXcgdGVz dHMgbmVlZGVkLgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclZpZGVvLmNwcDogQXR0ZW1w dCB0byB1c2Ugd2lkdGgvaGVpZ2h0IHByb3BlcnRpZXMKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRl clZpZGVvOjpSZW5kZXJWaWRlbyk6CisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclZpZGVvLmg6 IE1vcmUgYXBwcm9wcmlhdGUgY29uc3RydWN0b3Igc2lnbmF0dXJlCisKIDIwMTAtMDEtMjggIFN0 ZXZlIEJsb2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5 IERhdmlkIExldmluLgpJbmRleDogV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlkZW8uY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclZpZGVvLmNwcAkocmV2aXNpb24g NTQwMzkpCisrKyBXZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWRlby5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTUwLDE4ICs1MCwyNSBAQCB1c2luZyBuYW1lc3BhY2UgSFRNTE5hbWVzOwogc3RhdGlj IGNvbnN0IGludCBjRGVmYXVsdFdpZHRoID0gMzAwOwogc3RhdGljIGNvbnN0IGludCBjRGVmYXVs dEhlaWdodCA9IDE1MDsKIAotUmVuZGVyVmlkZW86OlJlbmRlclZpZGVvKEhUTUxNZWRpYUVsZW1l bnQqIHZpZGVvKQorUmVuZGVyVmlkZW86OlJlbmRlclZpZGVvKEhUTUxWaWRlb0VsZW1lbnQqIHZp ZGVvKQogICAgIDogUmVuZGVyTWVkaWEodmlkZW8pCiB7CiAgICAgaWYgKHZpZGVvLT5wbGF5ZXIo KSkKICAgICAgICAgc2V0SW50cmluc2ljU2l6ZSh2aWRlby0+cGxheWVyKCktPm5hdHVyYWxTaXpl KCkpOwogICAgIGVsc2UgewotICAgICAgICAvLyBWaWRlbyBpbiBzdGFuZGFsb25lIG1lZGlhIGRv Y3VtZW50cyBzaG91bGQgbm90IHVzZSB0aGUgZGVmYXVsdCAzMDB4MTUwCi0gICAgICAgIC8vIHNp emUgc2luY2UgdGhleSBhbHNvIGhhdmUgYXVkaW8gdGhyb3duIGF0IHRoZW0uIEJ5IHNldHRpbmcg dGhlIGludHJpbnNpYwotICAgICAgICAvLyBzaXplIHRvIDMwMHgxIHRoZSB2aWRlbyB3aWxsIHJl c2l6ZSBpdHNlbGYgaW4gdGhlc2UgY2FzZXMsIGFuZCBhdWRpbyB3aWxsCi0gICAgICAgIC8vIGhh dmUgdGhlIGNvcnJlY3QgaGVpZ2h0IChpdCBuZWVkcyB0byBiZSA+IDAgZm9yIGNvbnRyb2xzIHRv IHJlbmRlciBwcm9wZXJseSkuCi0gICAgICAgIGlmICh2aWRlby0+b3duZXJEb2N1bWVudCgpICYm IHZpZGVvLT5vd25lckRvY3VtZW50KCktPmlzTWVkaWFEb2N1bWVudCgpKQorICAgICAgICAvLyBX aGVuIHRoZSBuYXR1cmFsIHNpemUgb2YgdGhlIHZpZGVvIGlzIHVuYXZhaWxhYmxlLCB3ZSB1c2Ug dGhlIHByb3ZpZGVkCisgICAgICAgIC8vIHdpZHRoIGFuZCBoZWlnaHQgYXR0cmlidXRlcyBvZiB0 aGUgdmlkZW8gZWxlbWVudCBhcyB0aGUgaW50cmluc2ljIHNpemUgdW50aWwKKyAgICAgICAgLy8g YmV0dGVyIHZhbHVlcyBiZWNvbWUgYXZhaWxhYmxlLiBJZiB0aGVzZSBhdHRyaWJ1dGVzIGFyZSBu b3Qgc2V0LCB3ZSBmYWxsIGJhY2sKKyAgICAgICAgLy8gdG8gYSBkZWZhdWx0IHZpZGVvIHNpemUg KDMwMHgxNTApLgorICAgICAgICBpZiAodmlkZW8tPmhhc0F0dHJpYnV0ZSh3aWR0aEF0dHIpICYm IHZpZGVvLT5oYXNBdHRyaWJ1dGUoaGVpZ2h0QXR0cikpCisgICAgICAgICAgICBzZXRJbnRyaW5z aWNTaXplKEludFNpemUodmlkZW8tPndpZHRoKCksIHZpZGVvLT5oZWlnaHQoKSkpOworICAgICAg ICBlbHNlIGlmICh2aWRlby0+b3duZXJEb2N1bWVudCgpICYmIHZpZGVvLT5vd25lckRvY3VtZW50 KCktPmlzTWVkaWFEb2N1bWVudCgpKSB7CisgICAgICAgICAgICAvLyBWaWRlbyBpbiBzdGFuZGFs b25lIG1lZGlhIGRvY3VtZW50cyBzaG91bGQgbm90IHVzZSB0aGUgZGVmYXVsdCAzMDB4MTUwCisg ICAgICAgICAgICAvLyBzaXplIHNpbmNlIHRoZXkgYWxzbyBoYXZlIGF1ZGlvIHRocm93biBhdCB0 aGVtLiBCeSBzZXR0aW5nIHRoZSBpbnRyaW5zaWMKKyAgICAgICAgICAgIC8vIHNpemUgdG8gMzAw eDEgdGhlIHZpZGVvIHdpbGwgcmVzaXplIGl0c2VsZiBpbiB0aGVzZSBjYXNlcywgYW5kIGF1ZGlv IHdpbGwKKyAgICAgICAgICAgIC8vIGhhdmUgdGhlIGNvcnJlY3QgaGVpZ2h0IChpdCBuZWVkcyB0 byBiZSA+IDAgZm9yIGNvbnRyb2xzIHRvIHJlbmRlciBwcm9wZXJseSkuCiAgICAgICAgICAgICBz ZXRJbnRyaW5zaWNTaXplKEludFNpemUoY0RlZmF1bHRXaWR0aCwgMSkpOworICAgICAgICB9CiAg ICAgICAgIGVsc2UKICAgICAgICAgICAgIHNldEludHJpbnNpY1NpemUoSW50U2l6ZShjRGVmYXVs dFdpZHRoLCBjRGVmYXVsdEhlaWdodCkpOwogICAgIH0KSW5kZXg6IFdlYkNvcmUvcmVuZGVyaW5n L1JlbmRlclZpZGVvLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVmlk ZW8uaAkocmV2aXNpb24gNTQwMzkpCisrKyBXZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJWaWRlby5o CSh3b3JraW5nIGNvcHkpCkBAIC00MCw3ICs0MCw3IEBAIGNsYXNzIEdyYXBoaWNzTGF5ZXI7CiAK IGNsYXNzIFJlbmRlclZpZGVvIDogcHVibGljIFJlbmRlck1lZGlhIHsKIHB1YmxpYzoKLSAgICBS ZW5kZXJWaWRlbyhIVE1MTWVkaWFFbGVtZW50Kik7CisgICAgUmVuZGVyVmlkZW8oSFRNTFZpZGVv RWxlbWVudCopOwogICAgIHZpcnR1YWwgflJlbmRlclZpZGVvKCk7CiAKICAgICB2b2lkIHZpZGVv U2l6ZUNoYW5nZWQoKTsK