154007 2016-02-08 14:06:50 -0800 [GTK] Toggle buttons are blurry with GTK+ 3.19 2016-02-09 22:58:54 -0800 1 1 1 Unclassified WebKit WebKitGTK Other PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro webkit-unassigned bugs-noreply cgarcia commit-queue esprehn+autocc glenn kondapallykalyan mcatanzaro otte oldest_to_newest 1163007 0 mcatanzaro 2016-02-08 14:06:50 -0800 Since 3.19.7 we can get at min-width and min-height as style properties: https://git.gnome.org/browse/gtk+/commit/gtk/gtkcssstylepropertyimpl.c?id=ea69bf8c17dd5c9e7f76bf3cb4f56ec07b2e821f According to Company, we need to use these by default, rather than indicator-size. If either one is zero, ignore them and fall back to indicator-size. That should fix our blurry toggle buttons (checkbuttons, radio buttons). We should also audit all our other uses of style properties to make sure we're matching GTK+ 3.19. 1163175 1 270923 cgarcia 2016-02-09 06:06:33 -0800 Created attachment 270923 Patch 1163176 2 commit-queue 2016-02-09 06:08:27 -0800 Attachment 270923 did not pass style-queue: ERROR: Source/WebCore/rendering/RenderThemeGtk.cpp:551: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/rendering/RenderThemeGtk.cpp:598: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1163178 3 270923 mcatanzaro 2016-02-09 06:46:10 -0800 Comment on attachment 270923 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270923&action=review Thanks, I presume you tested it. > Source/WebCore/rendering/RenderThemeGtk.cpp:551 > + gtk_style_context_get(childContext.get(), gtk_style_context_get_state (childContext.get()), "min-width", &minWidth, "min-height", &minHeight, nullptr); Look what style checker caught: gtk_style_context_get_state ( > Source/WebCore/rendering/RenderThemeGtk.cpp:555 > + minSize.setHeight(minHeight); Company's suggestion was to do this: if (minWidth && minHeight) { minSize.setWidth(minWidth); minSize.setHeight(minHeight); } Any particular reason for not following that? > Source/WebCore/rendering/RenderThemeGtk.cpp:598 > + gtk_style_context_get(context.get(), gtk_style_context_get_state (context.get()), "min-width", &minWidth, "min-height", &minHeight, nullptr); Ditto > Source/WebCore/rendering/RenderThemeGtk.cpp:602 > + minSize.setHeight(minHeight); Ditto 1163179 4 otte 2016-02-09 06:50:51 -0800 (In reply to comment #3) > Company's suggestion was to do this: > > if (minWidth && minHeight) { > minSize.setWidth(minWidth); > minSize.setHeight(minHeight); > } > To be 100% absolutely exact, it's if (minWidth) minSize.setWidth(minWidth); if (minHeight) minSize.setHeight(minHeight); because that's what the code in https://git.gnome.org/browse/gtk+/tree/gtk/gtkbuiltinicon.c#n56 essentially does. 1163412 5 cgarcia 2016-02-09 22:56:25 -0800 (In reply to comment #4) > (In reply to comment #3) > > Company's suggestion was to do this: > > > > if (minWidth && minHeight) { > > minSize.setWidth(minWidth); > > minSize.setHeight(minHeight); > > } > > > To be 100% absolutely exact, it's > > if (minWidth) > minSize.setWidth(minWidth); > if (minHeight) > minSize.setHeight(minHeight); > > because that's what the code in > https://git.gnome.org/browse/gtk+/tree/gtk/gtkbuiltinicon.c#n56 essentially > does. Which is exactly what the patch does. 1163413 6 cgarcia 2016-02-09 22:58:54 -0800 Committed r196364: <http://trac.webkit.org/changeset/196364> 270923 2016-02-09 06:06:33 -0800 2016-02-09 06:46:10 -0800 Patch wcore-toggle-buttons.diff text/plain 3958 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBlZWI5MGU5Li5jYThlYjM0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTYtMDItMDkgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgorCisgICAgICAgIFtHVEtdIFRvZ2dsZSBidXR0b25zIGFyZSBibHVycnkgd2l0aCBHVEsrIDMu MTkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1NDAw NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFVzZSBt aW4td2lkdGgvbWluLWhlaWdodCBzdHlsZSBwcm9wZXJ0aWVzIHdoZW4gR1RLKyA+PSAzLjE5Ljcg dG8KKyAgICAgICAgZ2V0IHRoZSBzaXplIG9mIHRvZ2dsZSBidXR0b25zLgorCisgICAgICAgICog cmVuZGVyaW5nL1JlbmRlclRoZW1lR3RrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnNldFRvZ2ds ZVNpemUpOgorICAgICAgICAoV2ViQ29yZTo6cGFpbnRUb2dnbGUpOgorCiAyMDE2LTAyLTA0ICBD YXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBBZGQg YSB3YXkgdG8gdGVzdCBTY3JvbGxBbmltYXRvcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cmVuZGVyaW5nL1JlbmRlclRoZW1lR3RrLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9S ZW5kZXJUaGVtZUd0ay5jcHAKaW5kZXggM2JjMjJkNS4uYjhkNDQ3ZSAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lR3RrLmNwcAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVHdGsuY3BwCkBAIC01NDMsMTIgKzU0MywyMyBAQCBz dGF0aWMgdm9pZCBzZXRUb2dnbGVTaXplKFJlbmRlclRoZW1lUGFydCB0aGVtZVBhcnQsIFJlbmRl clN0eWxlJiBzdHlsZSkKICAgICAvLyBPdGhlciBwb3J0cyBoYXJkLWNvZGUgdGhpcyB0byAxMy4g R1RLKyB1c2VycyB0ZW5kIHRvIGRlbWFuZCB0aGUgbmF0aXZlIGxvb2suCiAgICAgZ2ludCBpbmRp Y2F0b3JTaXplOwogICAgIGd0a19zdHlsZV9jb250ZXh0X2dldF9zdHlsZShjb250ZXh0LmdldCgp LCAiaW5kaWNhdG9yLXNpemUiLCAmaW5kaWNhdG9yU2l6ZSwgbnVsbHB0cik7CisgICAgSW50U2l6 ZSBtaW5TaXplKGluZGljYXRvclNpemUsIGluZGljYXRvclNpemUpOworCisjaWYgR1RLX0NIRUNL X1ZFUlNJT04oMywgMTksIDcpCisgICAgR1JlZlB0cjxHdGtTdHlsZUNvbnRleHQ+IGNoaWxkQ29u dGV4dCA9IGNyZWF0ZVN0eWxlQ29udGV4dCh0aGVtZVBhcnQgPT0gQ2hlY2tCdXR0b24gPyBDaGVj a0J1dHRvbkNoZWNrIDogUmFkaW9CdXR0b25SYWRpbywgY29udGV4dC5nZXQoKSk7CisgICAgZ2lu dCBtaW5XaWR0aCwgbWluSGVpZ2h0OworICAgIGd0a19zdHlsZV9jb250ZXh0X2dldChjaGlsZENv bnRleHQuZ2V0KCksIGd0a19zdHlsZV9jb250ZXh0X2dldF9zdGF0ZSAoY2hpbGRDb250ZXh0Lmdl dCgpKSwgIm1pbi13aWR0aCIsICZtaW5XaWR0aCwgIm1pbi1oZWlnaHQiLCAmbWluSGVpZ2h0LCBu dWxscHRyKTsKKyAgICBpZiAobWluV2lkdGgpCisgICAgICAgIG1pblNpemUuc2V0V2lkdGgobWlu V2lkdGgpOworICAgIGlmIChtaW5IZWlnaHQpCisgICAgICAgIG1pblNpemUuc2V0SGVpZ2h0KG1p bkhlaWdodCk7CisjZW5kaWYKIAogICAgIGlmIChzdHlsZS53aWR0aCgpLmlzSW50cmluc2ljT3JB dXRvKCkpCi0gICAgICAgIHN0eWxlLnNldFdpZHRoKExlbmd0aChpbmRpY2F0b3JTaXplLCBGaXhl ZCkpOworICAgICAgICBzdHlsZS5zZXRXaWR0aChMZW5ndGgobWluU2l6ZS53aWR0aCgpLCBGaXhl ZCkpOwogCiAgICAgaWYgKHN0eWxlLmhlaWdodCgpLmlzQXV0bygpKQotICAgICAgICBzdHlsZS5z ZXRIZWlnaHQoTGVuZ3RoKGluZGljYXRvclNpemUsIEZpeGVkKSk7CisgICAgICAgIHN0eWxlLnNl dEhlaWdodChMZW5ndGgobWluU2l6ZS5oZWlnaHQoKSwgRml4ZWQpKTsKIH0KIAogc3RhdGljIHZv aWQgcGFpbnRUb2dnbGUoY29uc3QgUmVuZGVyVGhlbWVHdGsqIHRoZW1lLCBSZW5kZXJUaGVtZVBh cnQgdGhlbWVQYXJ0LCBjb25zdCBSZW5kZXJPYmplY3QmIHJlbmRlck9iamVjdCwgY29uc3QgUGFp bnRJbmZvJiBwYWludEluZm8sIGNvbnN0IEludFJlY3QmIGZ1bGxSZWN0KQpAQCAtNTgwLDE1ICs1 OTEsMjYgQEAgc3RhdGljIHZvaWQgcGFpbnRUb2dnbGUoY29uc3QgUmVuZGVyVGhlbWVHdGsqIHRo ZW1lLCBSZW5kZXJUaGVtZVBhcnQgdGhlbWVQYXJ0LAogICAgIC8vIGJ1dHRvbnMgdG8gYmUgYSBz bWFsbGVyIHNpemUgaXMgdGhhdCB3ZSBkb24ndCB3YW50IHRvIGJyZWFrIHNpdGUgbGF5b3V0cy4K ICAgICBnaW50IGluZGljYXRvclNpemU7CiAgICAgZ3RrX3N0eWxlX2NvbnRleHRfZ2V0X3N0eWxl KGNvbnRleHQuZ2V0KCksICJpbmRpY2F0b3Itc2l6ZSIsICZpbmRpY2F0b3JTaXplLCBudWxscHRy KTsKKyAgICBJbnRTaXplIG1pblNpemUoaW5kaWNhdG9yU2l6ZSwgaW5kaWNhdG9yU2l6ZSk7CisK KyNpZiBHVEtfQ0hFQ0tfVkVSU0lPTigzLCAxOSwgNykKKyAgICBnaW50IG1pbldpZHRoLCBtaW5I ZWlnaHQ7CisgICAgZ3RrX3N0eWxlX2NvbnRleHRfZ2V0KGNvbnRleHQuZ2V0KCksIGd0a19zdHls ZV9jb250ZXh0X2dldF9zdGF0ZSAoY29udGV4dC5nZXQoKSksICJtaW4td2lkdGgiLCAmbWluV2lk dGgsICJtaW4taGVpZ2h0IiwgJm1pbkhlaWdodCwgbnVsbHB0cik7CisgICAgaWYgKG1pbldpZHRo KQorICAgICAgICBtaW5TaXplLnNldFdpZHRoKG1pbldpZHRoKTsKKyAgICBpZiAobWluSGVpZ2h0 KQorICAgICAgICBtaW5TaXplLnNldEhlaWdodChtaW5IZWlnaHQpOworI2VuZGlmCisKICAgICBJ bnRSZWN0IHJlY3QoZnVsbFJlY3QpOwotICAgIGlmIChyZWN0LndpZHRoKCkgPiBpbmRpY2F0b3JT aXplKSB7Ci0gICAgICAgIHJlY3QuaW5mbGF0ZVgoLShyZWN0LndpZHRoKCkgLSBpbmRpY2F0b3JT aXplKSAvIDIpOwotICAgICAgICByZWN0LnNldFdpZHRoKGluZGljYXRvclNpemUpOyAvLyBJbiBj YXNlIHJlY3Qud2lkdGgoKSB3YXMgZXF1YWwgdG8gaW5kaWNhdG9yU2l6ZSArIDEuCisgICAgaWYg KHJlY3Qud2lkdGgoKSA+IG1pblNpemUud2lkdGgoKSkgeworICAgICAgICByZWN0LmluZmxhdGVY KC0ocmVjdC53aWR0aCgpIC0gbWluU2l6ZS53aWR0aCgpKSAvIDIpOworICAgICAgICByZWN0LnNl dFdpZHRoKG1pblNpemUud2lkdGgoKSk7IC8vIEluIGNhc2UgcmVjdC53aWR0aCgpIHdhcyBlcXVh bCB0byBtaW5TaXplLndpZHRoKCkgKyAxLgogICAgIH0KIAotICAgIGlmIChyZWN0LmhlaWdodCgp ID4gaW5kaWNhdG9yU2l6ZSkgewotICAgICAgICByZWN0LmluZmxhdGVZKC0ocmVjdC5oZWlnaHQo KSAtIGluZGljYXRvclNpemUpIC8gMik7Ci0gICAgICAgIHJlY3Quc2V0SGVpZ2h0KGluZGljYXRv clNpemUpOyAvLyBJbiBjYXNlIHJlY3QuaGVpZ2h0KCkgd2FzIGVxdWFsIHRvIGluZGljYXRvclNp emUgKyAxLgorICAgIGlmIChyZWN0LmhlaWdodCgpID4gbWluU2l6ZS5oZWlnaHQoKSkgeworICAg ICAgICByZWN0LmluZmxhdGVZKC0ocmVjdC5oZWlnaHQoKSAtIG1pblNpemUuaGVpZ2h0KCkpIC8g Mik7CisgICAgICAgIHJlY3Quc2V0SGVpZ2h0KG1pblNpemUuaGVpZ2h0KCkpOyAvLyBJbiBjYXNl IHJlY3QuaGVpZ2h0KCkgd2FzIGVxdWFsIHRvIG1pblNpemUuaGVpZ2h0KCkgKyAxLgogICAgIH0K IAogICAgIGd0a19yZW5kZXJfYmFja2dyb3VuZChjb250ZXh0LmdldCgpLCBwYWludEluZm8uY29u dGV4dCgpLnBsYXRmb3JtQ29udGV4dCgpLT5jcigpLCByZWN0LngoKSwgcmVjdC55KCksIHJlY3Qu d2lkdGgoKSwgcmVjdC5oZWlnaHQoKSk7Cg==