132883 2014-05-13 13:24:15 -0700 Update positioning/drawing of the image controls button 2014-05-13 14:46:20 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Mac All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson commit-queue esprehn+autocc glenn gyuyoung.kim kondapallykalyan thorton oldest_to_newest 1009052 0 beidson 2014-05-13 13:24:15 -0700 Update positioning/drawing of the image controls button <rdar://problem/16885077> 1009054 1 231404 beidson 2014-05-13 13:31:47 -0700 Created attachment 231404 Patch v1 1009056 2 231404 thorton 2014-05-13 13:36:09 -0700 Comment on attachment 231404 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=231404&action=review > Source/WebCore/html/shadow/mac/ImageControlsButtonElementMac.cpp:101 > + button->setAttribute(HTMLNames::styleAttr, String::format("right: %ipx; top: %ipx", positionOffset.width(), positionOffset.height())); Is this the normal way to set style attributes? I think you should be using setInlineStyleProperty for efficiency. See my use in PDFPluginTextAnnotation. > Source/WebCore/html/shadow/mac/imageControlsMac.css:-32 > - right: 18px; /* FIXME: Why is right: 0px off the right edge of the parent? */ we should keep a comment about this nonsense, because it's still wrong. just... above. 1009059 3 beidson 2014-05-13 13:44:16 -0700 https://trac.webkit.org/changeset/168728 1009064 4 thorton 2014-05-13 13:51:20 -0700 (In reply to comment #2) > (From update of attachment 231404 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=231404&action=review > > > Source/WebCore/html/shadow/mac/ImageControlsButtonElementMac.cpp:101 > > + button->setAttribute(HTMLNames::styleAttr, String::format("right: %ipx; top: %ipx", positionOffset.width(), positionOffset.height())); > > Is this the normal way to set style attributes? I think you should be using setInlineStyleProperty for efficiency. See my use in PDFPluginTextAnnotation. > > > Source/WebCore/html/shadow/mac/imageControlsMac.css:-32 > > - right: 18px; /* FIXME: Why is right: 0px off the right edge of the parent? */ > > we should keep a comment about this nonsense, because it's still wrong. just... above. I meant, to avoid String::formats :D setInlineStyleProperty(CSSPropertyTop, positionOffset.height(), CSS_PX); etc. 1009084 5 beidson 2014-05-13 14:46:20 -0700 Followed up in https://trac.webkit.org/changeset/168742 231404 2014-05-13 13:31:47 -0700 2014-05-13 13:36:09 -0700 Patch v1 patch text/plain 5352 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBhNTg4OTZiLi4zY2Y1ZjI4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjQg QEAKKzIwMTQtMDUtMTMgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAg ICAgIFVwZGF0ZSBwb3NpdGlvbmluZy9kcmF3aW5nIG9mIHRoZSBpbWFnZSBjb250cm9scyBidXR0 b24uCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNjg4NTA3Nz4gYW5kIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzI4ODMKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGh0bWwvc2hhZG93L21hYy9JbWFnZUNvbnRyb2xz QnV0dG9uRWxlbWVudE1hYy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpJbWFnZUNvbnRyb2xzQnV0 dG9uRWxlbWVudE1hYzo6bWF5YmVDcmVhdGUpOiBBZGQgc3R5bGUgYXR0cmlidXRlcyBmb3IgdG9w L3JpZ2h0CisgICAgICAgICAgcG9zaXRpb25pbmcgaGVyZSBiYXNlZCBvbiB0aGUgbWV0cmljcyBm cm9tIHRoZSByZW5kZXIgdGhlbWUuCisKKyAgICAgICAgKiBodG1sL3NoYWRvdy9tYWMvaW1hZ2VD b250cm9sc01hYy5jc3M6CisgICAgICAgICgueC13ZWJraXQtaW1hZ2UtY29udHJvbHMtYnV0dG9u KTogQ2Fu4oCZdCBoYXJkIGNvZGUgYW55IHBvc2l0aW9uaW5nLgorCisgICAgICAgICogcmVuZGVy aW5nL1JlbmRlclRoZW1lLmg6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZTo6aW1hZ2VD b250cm9sc0J1dHRvblBvc2l0aW9uT2Zmc2V0KToKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVy VGhlbWVNYWMuaDoKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyVGhlbWVNYWMubW06CisgICAg ICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZU1hYzo6c2VydmljZXNSb2xsb3ZlckJ1dHRvbkNlbGwp OgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVNYWM6OmltYWdlQ29udHJvbHNCdXR0b25Q b3NpdGlvbk9mZnNldCk6CisKIDIwMTQtMDUtMTMgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUu Y29tPgogCiAgICAgICAgIFRyeSB0byBmaXggdGhlICFFTkFCTEUoSUNPTkRBVEFCQVNFKSBidWls ZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9zaGFkb3cvbWFjL0ltYWdlQ29udHJv bHNCdXR0b25FbGVtZW50TWFjLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvc2hhZG93L21hYy9J bWFnZUNvbnRyb2xzQnV0dG9uRWxlbWVudE1hYy5jcHAKaW5kZXggNzU4ZGQyZi4uZGM0MzJhNCAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9zaGFkb3cvbWFjL0ltYWdlQ29udHJvbHNC dXR0b25FbGVtZW50TWFjLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3NoYWRvdy9tYWMv SW1hZ2VDb250cm9sc0J1dHRvbkVsZW1lbnRNYWMuY3BwCkBAIC05Nyw2ICs5Nyw5IEBAIFBhc3NS ZWZQdHI8SW1hZ2VDb250cm9sc0J1dHRvbkVsZW1lbnRNYWM+IEltYWdlQ29udHJvbHNCdXR0b25F bGVtZW50TWFjOjptYXliZUNyCiAgICAgUmVmUHRyPEltYWdlQ29udHJvbHNCdXR0b25FbGVtZW50 TWFjPiBidXR0b24gPSBhZG9wdFJlZihuZXcgSW1hZ2VDb250cm9sc0J1dHRvbkVsZW1lbnRNYWMo ZG9jdW1lbnQpKTsKICAgICBidXR0b24tPnNldEF0dHJpYnV0ZShIVE1MTmFtZXM6OmNsYXNzQXR0 ciwgIngtd2Via2l0LWltYWdlLWNvbnRyb2xzLWJ1dHRvbiIpOwogCisgICAgSW50U2l6ZSBwb3Np dGlvbk9mZnNldCA9IGRvY3VtZW50LnBhZ2UoKS0+dGhlbWUoKS5pbWFnZUNvbnRyb2xzQnV0dG9u UG9zaXRpb25PZmZzZXQoKTsKKyAgICBidXR0b24tPnNldEF0dHJpYnV0ZShIVE1MTmFtZXM6OnN0 eWxlQXR0ciwgU3RyaW5nOjpmb3JtYXQoInJpZ2h0OiAlaXB4OyB0b3A6ICVpcHgiLCBwb3NpdGlv bk9mZnNldC53aWR0aCgpLCBwb3NpdGlvbk9mZnNldC5oZWlnaHQoKSkpOworCiAgICAgcmV0dXJu IGJ1dHRvbi5yZWxlYXNlKCk7CiB9CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwv c2hhZG93L21hYy9pbWFnZUNvbnRyb2xzTWFjLmNzcyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvc2hh ZG93L21hYy9pbWFnZUNvbnRyb2xzTWFjLmNzcwppbmRleCAzNGQ4ODM0Li4zMzY4OTNlIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL3NoYWRvdy9tYWMvaW1hZ2VDb250cm9sc01hYy5j c3MKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC9zaGFkb3cvbWFjL2ltYWdlQ29udHJvbHNNYWMu Y3NzCkBAIC0yOSw3ICsyOSw2IEBACiAKIC54LXdlYmtpdC1pbWFnZS1jb250cm9scy1idXR0b24g ewogICAgIHBvc2l0aW9uOiBhYnNvbHV0ZTsKLSAgICByaWdodDogMThweDsgLyogRklYTUU6IFdo eSBpcyByaWdodDogMHB4IG9mZiB0aGUgcmlnaHQgZWRnZSBvZiB0aGUgcGFyZW50PyAqLwogCiAg ICAgLXdlYmtpdC1hcHBlYXJhbmNlOiBpbWFnZS1jb250cm9scy1idXR0b247CiAKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZS5oIGIvU291cmNlL1dlYkNv cmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lLmgKaW5kZXggZmY3NTNiNS4uMDIzOWYyOCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lLmgKKysrIGIvU291cmNl L1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lLmgKQEAgLTI1MCw2ICsyNTAsNyBAQCBwdWJs aWM6CiAKICNpZiBFTkFCTEUoU0VSVklDRV9DT05UUk9MUykKICAgICB2aXJ0dWFsIEludFNpemUg aW1hZ2VDb250cm9sc0J1dHRvblNpemUoY29uc3QgUmVuZGVyT2JqZWN0JikgY29uc3QgeyByZXR1 cm4gSW50U2l6ZSgpOyB9CisgICAgdmlydHVhbCBJbnRTaXplIGltYWdlQ29udHJvbHNCdXR0b25Q b3NpdGlvbk9mZnNldCgpIGNvbnN0IHsgcmV0dXJuIEludFNpemUoKTsgfQogI2VuZGlmCiAKIHBy b3RlY3RlZDoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVt ZU1hYy5oIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lTWFjLmgKaW5kZXgg OGQ3OTJiNi4uMTE0YTM2NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlclRoZW1lTWFjLmgKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1l TWFjLmgKQEAgLTIzMSw2ICsyMzEsOCBAQCBwcml2YXRlOgogI2lmIEVOQUJMRShTRVJWSUNFX0NP TlRST0xTKQogICAgIHZpcnR1YWwgYm9vbCBwYWludEltYWdlQ29udHJvbHNCdXR0b24oY29uc3Qg UmVuZGVyT2JqZWN0JiwgY29uc3QgUGFpbnRJbmZvJiwgY29uc3QgSW50UmVjdCYpIG92ZXJyaWRl OwogICAgIHZpcnR1YWwgSW50U2l6ZSBpbWFnZUNvbnRyb2xzQnV0dG9uU2l6ZShjb25zdCBSZW5k ZXJPYmplY3QmKSBjb25zdCBvdmVycmlkZTsKKyAgICB2aXJ0dWFsIEludFNpemUgaW1hZ2VDb250 cm9sc0J1dHRvblBvc2l0aW9uT2Zmc2V0KCkgY29uc3Qgb3ZlcnJpZGU7CisKICAgICBOU1NlcnZp Y2VzUm9sbG92ZXJCdXR0b25DZWxsICpzZXJ2aWNlc1JvbGxvdmVyQnV0dG9uQ2VsbCgpIGNvbnN0 OwogI2VuZGlmCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJU aGVtZU1hYy5tbSBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZU1hYy5tbQpp bmRleCBlM2RiODA3Li5mZDhhOGFkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJp bmcvUmVuZGVyVGhlbWVNYWMubW0KKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl clRoZW1lTWFjLm1tCkBAIC0yMDMwLDcgKzIwMzAsMTAgQEAgTlNTZXJ2aWNlc1JvbGxvdmVyQnV0 dG9uQ2VsbCogUmVuZGVyVGhlbWVNYWM6OnNlcnZpY2VzUm9sbG92ZXJCdXR0b25DZWxsKCkgY29u c3QKICNpZiBIQVZFKEFQUEtJVF9TRVJWSUNFX0NPTlRST0xTX1NVUFBPUlQpCiAgICAgaWYgKCFt X3NlcnZpY2VzUm9sbG92ZXJCdXR0b24pIHsKICAgICAgICAgbV9zZXJ2aWNlc1JvbGxvdmVyQnV0 dG9uID0gW05TU2VydmljZXNSb2xsb3ZlckJ1dHRvbkNlbGwgc2VydmljZVJvbGxvdmVyQnV0dG9u Q2VsbEZvclN0eWxlOk5TU2hhcmluZ1NlcnZpY2VQaWNrZXJTdHlsZVJvbGxvdmVyXTsKLSAgICAg ICAgW21fc2VydmljZXNSb2xsb3ZlckJ1dHRvbiBzZXRCb3JkZXJlZDpOT107CisgICAgICAgIFtt X3NlcnZpY2VzUm9sbG92ZXJCdXR0b24gc2V0QmV6ZWxTdHlsZTpOU1JvdW5kZWREaXNjbG9zdXJl QmV6ZWxTdHlsZV07CisgICAgICAgIFttX3NlcnZpY2VzUm9sbG92ZXJCdXR0b24gc2V0QnV0dG9u VHlwZTpOU1B1c2hPblB1c2hPZmZCdXR0b25dOworICAgICAgICBbbV9zZXJ2aWNlc1JvbGxvdmVy QnV0dG9uIHNldEltYWdlUG9zaXRpb246TlNJbWFnZU9ubHldOworICAgICAgICBbbV9zZXJ2aWNl c1JvbGxvdmVyQnV0dG9uIHNldFN0YXRlOk5PXTsKICAgICB9CiAKICAgICByZXR1cm4gbV9zZXJ2 aWNlc1JvbGxvdmVyQnV0dG9uLmdldCgpOwpAQCAtMjA3MSw2ICsyMDc0LDIxIEBAIEludFNpemUg UmVuZGVyVGhlbWVNYWM6OmltYWdlQ29udHJvbHNCdXR0b25TaXplKGNvbnN0IFJlbmRlck9iamVj dCYpIGNvbnN0CiAgICAgcmV0dXJuIEludFNpemUoKTsKICNlbmRpZgogfQorCitJbnRTaXplIFJl bmRlclRoZW1lTWFjOjppbWFnZUNvbnRyb2xzQnV0dG9uUG9zaXRpb25PZmZzZXQoKSBjb25zdAor eworI2lmIEhBVkUoQVBQS0lUX1NFUlZJQ0VfQ09OVFJPTFNfU1VQUE9SVCkKKyAgICAvLyBGSVhN RTogQ3VycmVudGx5IHRoZSBvZmZzZXRzIHdpbGwgYWx3YXlzIGJlIHRoZSBzYW1lIG5vIG1hdHRl ciB3aGF0IGltYWdlIHJlY3QgeW91IHRyeSB3aXRoLgorICAgIC8vIFRoaXMgbWF5IG5vdCBhbHdh eXMgYmUgdHJ1ZSBpbiB0aGUgZnV0dXJlLgorICAgIHN0YXRpYyBjb25zdCBpbnQgZHVtbXlEaW1l bnNpb24gPSAxMDA7CisgICAgSW50UmVjdCBkdW1teUltYWdlUmVjdCgwLCAwLCBkdW1teURpbWVu c2lvbiwgZHVtbXlEaW1lbnNpb24pOworICAgIE5TUmVjdCBib3VuZHMgPSBbc2VydmljZXNSb2xs b3ZlckJ1dHRvbkNlbGwoKSByZWN0Rm9yQm91bmRzOmR1bW15SW1hZ2VSZWN0IHByZWZlcnJlZEVk Z2U6TlNNaW5ZRWRnZV07CisKKyAgICByZXR1cm4gSW50U2l6ZShkdW1teURpbWVuc2lvbiAtIGJv dW5kcy5vcmlnaW4ueCwgYm91bmRzLm9yaWdpbi55KTsKKyNlbHNlCisgICAgcmV0dXJuIEludFNp emUoKTsKKyNlbmRpZgorfQogI2VuZGlmCiAKIH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK