151524 2015-11-20 15:56:52 -0800 [GTK] Off-by-one error in getStyleContext() 2015-11-21 09:00:10 -0800 1 1 1 Unclassified WebKit WebKitGTK Other PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro mcatanzaro bugs-noreply commit-queue esprehn+autocc glenn kondapallykalyan mcatanzaro oldest_to_newest 1143871 0 mcatanzaro 2015-11-20 15:56:52 -0800 In getStyleContext() in RenderThemeGtk.cpp: GtkWidgetPath* path = gtk_widget_path_new(); gtk_widget_path_append_type(path, widgetType); // ... gtk_widget_path_iter_add_class(path, 0, GTK_STYLE_CLASS_BUTTON); gtk_widget_path_iter_add_class(path, 1, "text-button"); Only one widget type was appended to the widget path, so the maximum valid index is 0. This code means to add both style classes to the first widget type in the widget path. This caused no bug in practice, because when the index is invalid, gtk_widget_path_iter_add_class() automatically changes the index to the last valid position in the widget path -- in this case, 0. 1143875 1 266010 mcatanzaro 2015-11-20 15:59:51 -0800 Created attachment 266010 Patch 1143942 2 266010 cgarcia 2015-11-21 01:44:21 -0800 Comment on attachment 266010 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266010&action=review Good catch, thanks! > Source/WebCore/rendering/RenderThemeGtk.cpp:178 > - gtk_widget_path_iter_add_class(path, 1, "text-button"); > + gtk_widget_path_iter_add_class(path, 0, "text-button"); I guess I understood this as the position of the class, rather than the widget at the given position. Since we always add one widget only, maybe it would be less confusing to use -1 in all cases. 1143947 3 mcatanzaro 2015-11-21 08:14:38 -0800 Now that I understand how the API works, using 0 seems better to me; I would use -1 when I don't know how long the widget path is. 1143952 4 266010 commit-queue 2015-11-21 09:00:06 -0800 Comment on attachment 266010 Patch Clearing flags on attachment: 266010 Committed r192724: <http://trac.webkit.org/changeset/192724> 1143953 5 commit-queue 2015-11-21 09:00:10 -0800 All reviewed patches have been landed. Closing bug. 266010 2015-11-20 15:59:51 -0800 2015-11-21 09:00:06 -0800 Patch bug-151524-20151120175938.patch text/plain 2400 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMTkyNjg1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODExOWFkYTlmZjE1MDA4 NWM4ZDc2YzhjM2MyYWY3MzNlNWVmODllMy4uODQwYjdlNTQwZGM0ZTI3ZDdlNmZlOWM3MDJhODJj YTk5YjEzNTA5MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI4IEBACisyMDE1LTExLTIwICBNaWNo YWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXSBP ZmYtYnktb25lIGVycm9yIGluIGdldFN0eWxlQ29udGV4dCgpCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTE1MjQKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBHdGtXaWRnZXRQYXRoKiBwYXRoID0gZ3RrX3dp ZGdldF9wYXRoX25ldygpOworICAgICAgICBndGtfd2lkZ2V0X3BhdGhfYXBwZW5kX3R5cGUocGF0 aCwgd2lkZ2V0VHlwZSk7CisgICAgICAgIC8vIC4uLgorICAgICAgICBndGtfd2lkZ2V0X3BhdGhf aXRlcl9hZGRfY2xhc3MocGF0aCwgMCwgR1RLX1NUWUxFX0NMQVNTX0JVVFRPTik7CisgICAgICAg IGd0a193aWRnZXRfcGF0aF9pdGVyX2FkZF9jbGFzcyhwYXRoLCAxLCAidGV4dC1idXR0b24iKTsK KworICAgICAgICBPbmx5IG9uZSB3aWRnZXQgdHlwZSB3YXMgYXBwZW5kZWQgdG8gdGhlIHdpZGdl dCBwYXRoLCBzbyB0aGUgbWF4aW11bSB2YWxpZCBpbmRleCBpcyAwLiBUaGlzCisgICAgICAgIGNv ZGUgbWVhbnMgdG8gYWRkIGJvdGggc3R5bGUgY2xhc3NlcyB0byB0aGUgZmlyc3Qgd2lkZ2V0IHR5 cGUgaW4gdGhlIHdpZGdldCBwYXRoLCBzbyB0aGUKKyAgICAgICAgc2Vjb25kIGNhbGwgc2hvdWxk IHVzZSBpbmRleCAwIHJhdGhlciB0aGFuIGluZGV4IDEuCisKKyAgICAgICAgVGhpcyBjYXVzZWQg bm8gYnVnIGluIHByYWN0aWNlLCBiZWNhdXNlIHdoZW4gdGhlIGluZGV4IGlzIGludmFsaWQsCisg ICAgICAgIGd0a193aWRnZXRfcGF0aF9pdGVyX2FkZF9jbGFzcygpIGF1dG9tYXRpY2FsbHkgY2hh bmdlcyB0aGUgaW5kZXggdG8gdGhlIGxhc3QgdmFsaWQgcG9zaXRpb24KKyAgICAgICAgaW4gdGhl IHdpZGdldCBwYXRoIC0tIGluIHRoaXMgY2FzZSwgMC4gVGhpcyBpcyByb3V0aW5lbHkgZG9uZSB3 aXRoIC0xIGFzIGEgY29udmVudGlvbiBmb3IKKyAgICAgICAgc3BlY2lmeWluZyB0aGUgbGFzdCBw b3NpdGlvbiBpbiB0aGUgd2lkZ2V0IHBhdGguCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVy VGhlbWVHdGsuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Z2V0U3R5bGVDb250ZXh0KToKKwogMjAx NS0xMS0yMCAgWW91ZW5uIEZhYmxldCAgPHlvdWVubi5mYWJsZXRAY3JmLmNhbm9uLmZyPgogCiAg ICAgICAgIFVzZSBIVFRQSGVhZGVyTmFtZSBhcyBtdWNoIGFzIHBvc3NpYmxlIGluIFhNTEh0dHBS ZXF1ZXN0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVH dGsuY3BwIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lR3RrLmNwcAppbmRl eCA0YjYyZTJkYTYxYjIwOGQ3MGQ5MzViMzFhZGIzZGVmMjkzMGJjZWU3Li4xYWZiMTE0OTI1NTY4 OGM2ODBhYWY4NWZjZjA0NGQ3MDI4NjBiYjQ2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9y ZW5kZXJpbmcvUmVuZGVyVGhlbWVHdGsuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJUaGVtZUd0ay5jcHAKQEAgLTE3NSw3ICsxNzUsNyBAQCBzdGF0aWMgR3RrU3R5bGVD b250ZXh0KiBnZXRTdHlsZUNvbnRleHQoR1R5cGUgd2lkZ2V0VHlwZSkKICAgICAgICAgZ3RrX3dp ZGdldF9wYXRoX2l0ZXJfYWRkX2NsYXNzKHBhdGgsIDAsICJhcnJvdyIpOwogICAgIGVsc2UgaWYg KHdpZGdldFR5cGUgPT0gR1RLX1RZUEVfQlVUVE9OKSB7CiAgICAgICAgIGd0a193aWRnZXRfcGF0 aF9pdGVyX2FkZF9jbGFzcyhwYXRoLCAwLCBHVEtfU1RZTEVfQ0xBU1NfQlVUVE9OKTsKLSAgICAg ICAgZ3RrX3dpZGdldF9wYXRoX2l0ZXJfYWRkX2NsYXNzKHBhdGgsIDEsICJ0ZXh0LWJ1dHRvbiIp OworICAgICAgICBndGtfd2lkZ2V0X3BhdGhfaXRlcl9hZGRfY2xhc3MocGF0aCwgMCwgInRleHQt YnV0dG9uIik7CiAgICAgfSBlbHNlIGlmICh3aWRnZXRUeXBlID09IEdUS19UWVBFX1NDQUxFKQog ICAgICAgICBndGtfd2lkZ2V0X3BhdGhfaXRlcl9hZGRfY2xhc3MocGF0aCwgMCwgR1RLX1NUWUxF X0NMQVNTX1NDQUxFKTsKICAgICBlbHNlIGlmICh3aWRnZXRUeXBlID09IEdUS19UWVBFX1NFUEFS QVRPUikK