94625 2012-08-21 12:15:35 -0700 Create CSS color output string on 8 bits 2012-08-21 17:19:02 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 benjamin benjamin cmarcelo darin macpherson menard webkit.review.bot oldest_to_newest 700987 0 benjamin 2012-08-21 12:15:35 -0700 Forcing 16 bits on everyone is bad. 700994 1 159735 benjamin 2012-08-21 12:23:04 -0700 Created attachment 159735 Patch 701059 2 159735 kling 2012-08-21 13:26:13 -0700 Comment on attachment 159735 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159735&action=review r=me > Source/WebCore/css/CSSPrimitiveValue.cpp:1011 > + bool colorHasAlpha = color.hasAlpha(); Does this really need a local variable? > Source/WebCore/css/CSSPrimitiveValue.cpp:1029 > + NumberToStringBuffer buffer; > + const char* alphaString = numberToFixedPrecisionString(color.alpha() / 255.0f, 6, buffer, true); > + result.append(alphaString, strlen(alphaString)); Too bad we need the strlen() here. 701123 3 159735 benjamin 2012-08-21 14:22:38 -0700 Comment on attachment 159735 Patch Clearing flags on attachment: 159735 Committed r126186: <http://trac.webkit.org/changeset/126186> 701124 4 benjamin 2012-08-21 14:22:40 -0700 All reviewed patches have been landed. Closing bug. 701134 5 darin 2012-08-21 14:35:56 -0700 Shouldn’t we be doing this with StringBuilder rather than Vector<LChar>? 701352 6 benjamin 2012-08-21 17:19:02 -0700 (In reply to comment #5) > Shouldn’t we be doing this with StringBuilder rather than Vector<LChar>? StringBuilder could do the job but it does not anything similar to WTF::appendNumber(). I do not mind updating StringBuilder if you think it is important. 159735 2012-08-21 12:23:04 -0700 2012-08-21 14:22:38 -0700 Patch bug-94625-20120821122303.patch text/plain 3605 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTI2MTc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzQ1OGJmMmNlZTU0NjQ2 M2E4NGI0M2I2N2NkNDlhY2E1OTMyYWI1Ni4uMTMxODU3YzVhYTAxYzQyNzlmOTc2ODVlZmZkYzc1 NDRmZjg1Yjk4OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA4LTIxICBCZW5q YW1pbiBQb3VsYWluICA8YnBvdWxhaW5AYXBwbGUuY29tPgorCisgICAgICAgIENyZWF0ZSBDU1Mg Y29sb3Igb3V0cHV0IHN0cmluZyBvbiA4IGJpdHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTk0NjI1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgKiBjc3MvQ1NTUHJpbWl0aXZlVmFsdWUuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6Q1NTUHJpbWl0aXZlVmFsdWU6OmN1c3RvbUNzc1RleHQpOgorICAgICAgICBQ cmV2aW91c2x5LCB0aGUgb3V0cHV0IHN0cmluZyBmb3IgYSBDU1MgY29sb3Igd2FzIGNvbXB1dGVk IG9uIDE2IGJpdHMuCisgICAgICAgIFRoaXMgd2FzIG1haW5seSBmb3JjZWQgYnkgdGhlIHVzZSBv ZiBTdHJpbmc6Om51bWJlcigpLgorCisgICAgICAgIFNpbmNlIHRoZSBkb3VibGUgdG8gc3RyaW5n IGNvbnZlcnNpb24gaXMgZG9uZSBvbiA4Yml0cyBhbnl3YXksIEkgY2hhbmdlZAorICAgICAgICB0 aGUgY29kZSB0byB1c2UgZHRvYSdzIG51bWJlclRvRml4ZWRQcmVjaXNpb25TdHJpbmcgZGlyZWN0 bHkgaW5zdGVhZCBvZgorICAgICAgICBTdHJpbmc6Om51bWJlcigpLgorICAgICAgICBBbGwgdGhl IG90aGVyIHBhcnRzIHdlcmUgYWxyZWFkeSBvbiA4Yml0cy4KKwogMjAxMi0wOC0yMSAgUGF0cmlj ayBHYW5zdGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBbV0lOXSBCdWlsZCBm aXggZm9yICFFTkFCTEUoRFJBR19TVVBQT1JUKS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L2Nzcy9DU1NQcmltaXRpdmVWYWx1ZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTUHJpbWl0 aXZlVmFsdWUuY3BwCmluZGV4IGRjNTdmYThmMzcwNGZiMDI0NDQ0ZTM3NzBhNWQ2OTc3ZGVhN2Zj NDAuLjdkMWNhMjk3NDIwZWY2ZjlhNTdiNTI1OTBkNWE2YmY0YTM5OWEzYTIgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQcmltaXRpdmVWYWx1ZS5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvY3NzL0NTU1ByaW1pdGl2ZVZhbHVlLmNwcApAQCAtMSw2ICsxLDYgQEAKIC8qCiAgKiAo QykgMTk5OS0yMDAzIExhcnMgS25vbGwgKGtub2xsQGtkZS5vcmcpCi0gKiBDb3B5cmlnaHQgKEMp IDIwMDQsIDIwMDUsIDIwMDYsIDIwMDcsIDIwMDggQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2Vy dmVkLgorICogQ29weXJpZ2h0IChDKSAyMDA0LCAyMDA1LCAyMDA2LCAyMDA3LCAyMDA4LCAyMDEy IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KICAqCiAgKiBUaGlzIGxpYnJhcnkgaXMg ZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUgaXQgYW5kL29yCiAgKiBtb2RpZnkg aXQgdW5kZXIgdGhlIHRlcm1zIG9mIHRoZSBHTlUgTGlicmFyeSBHZW5lcmFsIFB1YmxpYwpAQCAt MTAwMSwzMiArMTAwMSwzMiBAQCBTdHJpbmcgQ1NTUHJpbWl0aXZlVmFsdWU6OmN1c3RvbUNzc1Rl eHQoKSBjb25zdAogICAgICAgICB9CiAgICAgICAgIGNhc2UgQ1NTX1JHQkNPTE9SOgogICAgICAg ICBjYXNlIENTU19QQVJTRVJfSEVYQ09MT1I6IHsKLSAgICAgICAgICAgIERFRklORV9TVEFUSUNf TE9DQUwoY29uc3QgU3RyaW5nLCBjb21tYVNwYWNlLCAoIiwgIikpOwotICAgICAgICAgICAgREVG SU5FX1NUQVRJQ19MT0NBTChjb25zdCBTdHJpbmcsIHJnYlBhcmVuLCAoInJnYigiKSk7Ci0gICAg ICAgICAgICBERUZJTkVfU1RBVElDX0xPQ0FMKGNvbnN0IFN0cmluZywgcmdiYVBhcmVuLCAoInJn YmEoIikpOwotCiAgICAgICAgICAgICBSR0JBMzIgcmdiQ29sb3IgPSBtX3ZhbHVlLnJnYmNvbG9y OwogICAgICAgICAgICAgaWYgKG1fcHJpbWl0aXZlVW5pdFR5cGUgPT0gQ1NTX1BBUlNFUl9IRVhD T0xPUikKICAgICAgICAgICAgICAgICBDb2xvcjo6cGFyc2VIZXhDb2xvcihtX3ZhbHVlLnN0cmlu ZywgcmdiQ29sb3IpOwogICAgICAgICAgICAgQ29sb3IgY29sb3IocmdiQ29sb3IpOwogCi0gICAg ICAgICAgICBWZWN0b3I8VUNoYXI+IHJlc3VsdDsKKyAgICAgICAgICAgIFZlY3RvcjxMQ2hhcj4g cmVzdWx0OwogICAgICAgICAgICAgcmVzdWx0LnJlc2VydmVJbml0aWFsQ2FwYWNpdHkoMzIpOwot ICAgICAgICAgICAgaWYgKGNvbG9yLmhhc0FscGhhKCkpCi0gICAgICAgICAgICAgICAgYXBwZW5k KHJlc3VsdCwgcmdiYVBhcmVuKTsKKyAgICAgICAgICAgIGJvb2wgY29sb3JIYXNBbHBoYSA9IGNv bG9yLmhhc0FscGhhKCk7CisgICAgICAgICAgICBpZiAoY29sb3JIYXNBbHBoYSkKKyAgICAgICAg ICAgICAgICByZXN1bHQuYXBwZW5kKCJyZ2JhKCIsIDUpOwogICAgICAgICAgICAgZWxzZQotICAg ICAgICAgICAgICAgIGFwcGVuZChyZXN1bHQsIHJnYlBhcmVuKTsKKyAgICAgICAgICAgICAgICBy ZXN1bHQuYXBwZW5kKCJyZ2IoIiwgNCk7CiAKICAgICAgICAgICAgIGFwcGVuZE51bWJlcihyZXN1 bHQsIHN0YXRpY19jYXN0PHVuc2lnbmVkIGNoYXI+KGNvbG9yLnJlZCgpKSk7Ci0gICAgICAgICAg ICBhcHBlbmQocmVzdWx0LCBjb21tYVNwYWNlKTsKKyAgICAgICAgICAgIHJlc3VsdC5hcHBlbmQo IiwgIiwgMik7CiAKICAgICAgICAgICAgIGFwcGVuZE51bWJlcihyZXN1bHQsIHN0YXRpY19jYXN0 PHVuc2lnbmVkIGNoYXI+KGNvbG9yLmdyZWVuKCkpKTsKLSAgICAgICAgICAgIGFwcGVuZChyZXN1 bHQsIGNvbW1hU3BhY2UpOworICAgICAgICAgICAgcmVzdWx0LmFwcGVuZCgiLCAiLCAyKTsKIAog ICAgICAgICAgICAgYXBwZW5kTnVtYmVyKHJlc3VsdCwgc3RhdGljX2Nhc3Q8dW5zaWduZWQgY2hh cj4oY29sb3IuYmx1ZSgpKSk7Ci0gICAgICAgICAgICBpZiAoY29sb3IuaGFzQWxwaGEoKSkgewot ICAgICAgICAgICAgICAgIGFwcGVuZChyZXN1bHQsIGNvbW1hU3BhY2UpOwotICAgICAgICAgICAg ICAgIGFwcGVuZChyZXN1bHQsIFN0cmluZzo6bnVtYmVyKGNvbG9yLmFscGhhKCkgLyAyNTUuMGYp KTsKKyAgICAgICAgICAgIGlmIChjb2xvckhhc0FscGhhKSB7CisgICAgICAgICAgICAgICAgcmVz dWx0LmFwcGVuZCgiLCAiLCAyKTsKKworICAgICAgICAgICAgICAgIE51bWJlclRvU3RyaW5nQnVm ZmVyIGJ1ZmZlcjsKKyAgICAgICAgICAgICAgICBjb25zdCBjaGFyKiBhbHBoYVN0cmluZyA9IG51 bWJlclRvRml4ZWRQcmVjaXNpb25TdHJpbmcoY29sb3IuYWxwaGEoKSAvIDI1NS4wZiwgNiwgYnVm ZmVyLCB0cnVlKTsKKyAgICAgICAgICAgICAgICByZXN1bHQuYXBwZW5kKGFscGhhU3RyaW5nLCBz dHJsZW4oYWxwaGFTdHJpbmcpKTsKICAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgcmVzdWx0 LmFwcGVuZCgnKScpOwo=