60372 2011-05-06 07:34:57 -0700 [Qt] tst_QWebElement::style() fails because QWebElement::InlineStyle now works as expected 2012-11-12 07:31:15 -0800 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Qt P2 Normal --- 38654 1 ademar jturcotte abecsi cmarcelo commit-queue jturcotte ossy rafael.lobo tonikitoo oldest_to_newest 399505 0 ademar 2011-05-06 07:34:57 -0700 The QWebElement documentation says that about QWebElement::InlineStyle: "Return the property value as it is defined in the element, without respecting style inheritance and other CSS rules". In our QWebElement test, we do the following: """ ... p.setStyleProperty("color", "green !important"); QCOMPARE(p.styleProperty("color", QWebElement::InlineStyle), QLatin1String("green")); QCOMPARE(p.styleProperty("color", QWebElement::CascadedStyle), QLatin1String("green")); p.setStyleProperty("color", "blue"); QCOMPARE(p.styleProperty("color", QWebElement::InlineStyle), QLatin1String("blue")); ... """ But the test fails: """ FAIL! : tst_QWebElement::style() Compared values are not the same Actual (p.styleProperty("color", QWebElement::InlineStyle)): green Expected (QLatin1String("blue")): blue Loc: [/opt/projects/webkit/webkit/Source/WebKit/qt/tests/qwebelement/tst_qwebelement.cpp(486)] """ When this was first implemented, the test expectation was wrong and thus the test was passing. Then there was a change in the CSS code that introduced a regression that curiously made our implementation actually work (and the test fail). The test was fixed and right after so was the regression, so now we have a broken test (as it should be). See Bug 58032 for the whole story. 399678 1 ademar 2011-05-06 11:43:45 -0700 From WebCore::CSSMutableStyleDeclaration::setProperty(): """ // When replacing an existing property value, this moves the property to the end of the list. """ (it also says "Firefox preserves the position, and MSIE moves the property to the beginning", but that's not relevant for this bug) In the testing code, we set the property twice for the same element: p.setStyleProperty("color", "green !important"); ... p.setStyleProperty("color", "blue"); ... So when we call styleProperty(QWebElement::InlineStyle) after the second setStyleProperty(), there are two inline styles and we get the one with high priority. In summary, the implementation is right and the test is wrong, so it's just a matter or reverting the previous "fix". 399788 2 ademar 2011-05-06 13:52:15 -0700 Reopening, please ignore my previous round of comments. :-P I'll make sure my patch includes better documentation to avoid more confusion. 407059 3 94212 ossy 2011-05-20 06:53:01 -0700 Created attachment 94212 mark failing test case as expected fail 407063 4 94212 ademar 2011-05-20 06:56:08 -0700 Comment on attachment 94212 mark failing test case as expected fail LGTM. Fixing this will take some more time (time that I don't have at the moment) :( 407085 5 94212 commit-queue 2011-05-20 07:29:28 -0700 Comment on attachment 94212 mark failing test case as expected fail Clearing flags on attachment: 94212 Committed r86953: <http://trac.webkit.org/changeset/86953> 407086 6 commit-queue 2011-05-20 07:29:33 -0700 All reviewed patches have been landed. Closing bug. 407087 7 ossy 2011-05-20 07:30:34 -0700 Reopen to fix it. 413583 8 ademar 2011-06-01 12:32:57 -0700 Revision r86953 cherry-picked into qtwebkit-2.2 with commit b6ea329 <http://gitorious.org/webkit/qtwebkit/commit/b6ea329> 763043 9 jturcotte 2012-11-09 08:32:22 -0800 Ademar was right in comment #1, a non-important inline style shouldn't be overwritten by a non-important one. So InlineStyle doesn't apply inheritance, but does consider the importance. 763044 10 173313 jturcotte 2012-11-09 08:35:16 -0800 Created attachment 173313 Patch Unmark the failure and fix the expected value. 764401 11 jturcotte 2012-11-12 07:31:15 -0800 Committed r134233: <http://trac.webkit.org/changeset/134233> 94212 2011-05-20 06:53:01 -0700 2011-05-20 07:30:51 -0700 mark failing test case as expected fail 1.patch text/plain 1459 ossy ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9x dC9DaGFuZ2VMb2cKaW5kZXggYzdjZTg1Yi4uZDczOTIwMSAxMDA2NDQKLS0tIGEvU291cmNlL1dl YktpdC9xdC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9xdC9DaGFuZ2VMb2cKQEAgLTEs NSArMSwxNSBAQAogMjAxMS0wNS0yMCAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8b3NzeUB3ZWJraXQu b3JnPgogCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtR dF0gdHN0X1FXZWJFbGVtZW50OjpzdHlsZSgpIGZhaWxzIGJlY2F1c2UgUVdlYkVsZW1lbnQ6Oklu bGluZVN0eWxlIGRvZXNuJ3Qgd29yayBhcyBleHBlY3RlZAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjAzNzIKKworICAgICAgICAqIHRlc3RzL3F3ZWJl bGVtZW50L3RzdF9xd2ViZWxlbWVudC5jcHA6IE1hcmsgZmFpbGluZyB0ZXN0IGNhc2UgYXMgZXhw ZWN0ZWQgZmFpbC4KKyAgICAgICAgKHRzdF9RV2ViRWxlbWVudDo6c3R5bGUpOgorCisyMDExLTA1 LTIwICBDc2FiYSBPc3p0cm9nb27DoWMgIDxvc3N5QHdlYmtpdC5vcmc+CisKICAgICAgICAgUmV2 aWV3ZWQgYnkgQmVuamFtaW4gUG91bGFpbi4KIAogICAgICAgICBbUXRdRml4IHRzdF9RV2ViRnJh bWU6OnNldFVybFRvSW52YWxpZCgpIGF1dG90ZXN0IGFmdGVyIHI4NDc2MgpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYktpdC9xdC90ZXN0cy9xd2ViZWxlbWVudC90c3RfcXdlYmVsZW1lbnQuY3BwIGIv U291cmNlL1dlYktpdC9xdC90ZXN0cy9xd2ViZWxlbWVudC90c3RfcXdlYmVsZW1lbnQuY3BwCmlu ZGV4IDllOTk0OGEuLjBmMDViMmIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvcXQvdGVzdHMv cXdlYmVsZW1lbnQvdHN0X3F3ZWJlbGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L3F0L3Rl c3RzL3F3ZWJlbGVtZW50L3RzdF9xd2ViZWxlbWVudC5jcHAKQEAgLTQ4Myw2ICs0ODMsNyBAQCB2 b2lkIHRzdF9RV2ViRWxlbWVudDo6c3R5bGUoKQogICAgIFFDT01QQVJFKHAuc3R5bGVQcm9wZXJ0 eSgiY29sb3IiLCBRV2ViRWxlbWVudDo6Q2FzY2FkZWRTdHlsZSksIFFMYXRpbjFTdHJpbmcoImdy ZWVuIikpOwogCiAgICAgcC5zZXRTdHlsZVByb3BlcnR5KCJjb2xvciIsICJibHVlIik7CisgICAg UUVYUEVDVF9GQUlMKCIiLCAiaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTYwMzcyIiwgQ29udGludWUpOwogICAgIFFDT01QQVJFKHAuc3R5bGVQcm9wZXJ0eSgiY29sb3Ii LCBRV2ViRWxlbWVudDo6SW5saW5lU3R5bGUpLCBRTGF0aW4xU3RyaW5nKCJibHVlIikpOwogICAg IFFDT01QQVJFKHAuc3R5bGVQcm9wZXJ0eSgiY29sb3IiLCBRV2ViRWxlbWVudDo6Q2FzY2FkZWRT dHlsZSksIFFMYXRpbjFTdHJpbmcoImdyZWVuIikpOwogCg== 173313 2012-11-09 08:35:16 -0800 2012-11-12 06:33:00 -0800 Patch bug-60372-20121109173323.patch text/plain 2050 jturcotte U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzMjg1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L3F0 L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCmluZGV4IDA3Y2U5YWVkZjQ5 YmUxMTlmN2Y0YTFmOGQ5M2Q1OGU1N2ZkOWE2NzkuLmVmZTdlN2E2Y2MyOTc3ZDc2ZDNlNzA2MDU0 M2U0OTUzZDg0YjExZmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCisr KyBiL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTggQEAKIDIwMTItMTEt MDkgIEpvY2VseW4gVHVyY290dGUgIDxqb2NlbHluLnR1cmNvdHRlQGRpZ2lhLmNvbT4KIAorICAg ICAgICBbUXRdIHRzdF9RV2ViRWxlbWVudDo6c3R5bGUoKSBmYWlscyBiZWNhdXNlIFFXZWJFbGVt ZW50OjpJbmxpbmVTdHlsZSBub3cgd29ya3MgYXMgZXhwZWN0ZWQKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYwMzcyCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVXBkYXRlIHRoZSBleHBlY3RlZCB2YWx1ZSB0 byBmb2xsb3cgdGhpcyBjb21tZW50IGluIFN0eWxlUHJvcGVydHlTZXQ6OmFkZFBhcnNlZFByb3Bl cnR5OgorICAgICAgICAiT25seSBhZGQgcHJvcGVydGllcyB0aGF0IGhhdmUgbm8gIWltcG9ydGFu dCBjb3VudGVycGFydCBwcmVzZW50IgorCisgICAgICAgICogdGVzdHMvcXdlYmVsZW1lbnQvdHN0 X3F3ZWJlbGVtZW50LmNwcDoKKyAgICAgICAgKHRzdF9RV2ViRWxlbWVudDo6c3R5bGUpOgorCisy MDEyLTExLTA5ICBKb2NlbHluIFR1cmNvdHRlICA8am9jZWx5bi50dXJjb3R0ZUBkaWdpYS5jb20+ CisKICAgICAgICAgW1F0XSB0c3RfUVdlYkVsZW1lbnQ6OnN0eWxlKCkgZmFpbHMgYmVjYXVzZSBR V2ViRWxlbWVudDo6Q2FzY2FkZWRTdHlsZSBkb2Vzbid0IHdvcmsgYXMgZXhwZWN0ZWQKICAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY1MjQ0CiAKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQvcXQvdGVzdHMvcXdlYmVsZW1lbnQvdHN0X3F3ZWJlbGVtZW50 LmNwcCBiL1NvdXJjZS9XZWJLaXQvcXQvdGVzdHMvcXdlYmVsZW1lbnQvdHN0X3F3ZWJlbGVtZW50 LmNwcAppbmRleCAwZGM2NTQ2MzIwYWI5ZmRhYTQ4MzE3NDUwOTY5NGU5NDE3YWYzNzAwLi5lN2M4 ZTE4NDQxMDExODM2NzBmNTI5OGIxNDhjMjliZTljZWFmMjQ4IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L3F0L3Rlc3RzL3F3ZWJlbGVtZW50L3RzdF9xd2ViZWxlbWVudC5jcHAKKysrIGIvU291 cmNlL1dlYktpdC9xdC90ZXN0cy9xd2ViZWxlbWVudC90c3RfcXdlYmVsZW1lbnQuY3BwCkBAIC00 ODMsOCArNDgzLDggQEAgdm9pZCB0c3RfUVdlYkVsZW1lbnQ6OnN0eWxlKCkKICAgICBRQ09NUEFS RShwLnN0eWxlUHJvcGVydHkoImNvbG9yIiwgUVdlYkVsZW1lbnQ6OkNhc2NhZGVkU3R5bGUpLCBR TGF0aW4xU3RyaW5nKCJncmVlbiIpKTsKIAogICAgIHAuc2V0U3R5bGVQcm9wZXJ0eSgiY29sb3Ii LCAiYmx1ZSIpOwotICAgIFFFWFBFQ1RfRkFJTCgiIiwgImh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD02MDM3MiIsIENvbnRpbnVlKTsKLSAgICBRQ09NUEFSRShwLnN0eWxl UHJvcGVydHkoImNvbG9yIiwgUVdlYkVsZW1lbnQ6OklubGluZVN0eWxlKSwgUUxhdGluMVN0cmlu ZygiYmx1ZSIpKTsKKyAgICAvLyBBIGN1cnJlbnQgaW1wb3J0YW50IElubGluZVN0eWxlIHNob3Vs ZG4ndCBiZSBvdmVyd3JpdHRlbiBieSBhIG5vbi1pbXBvcnRhbnQgb25lLgorICAgIFFDT01QQVJF KHAuc3R5bGVQcm9wZXJ0eSgiY29sb3IiLCBRV2ViRWxlbWVudDo6SW5saW5lU3R5bGUpLCBRTGF0 aW4xU3RyaW5nKCJncmVlbiIpKTsKICAgICBRQ09NUEFSRShwLnN0eWxlUHJvcGVydHkoImNvbG9y IiwgUVdlYkVsZW1lbnQ6OkNhc2NhZGVkU3R5bGUpLCBRTGF0aW4xU3RyaW5nKCJncmVlbiIpKTsK IAogICAgIHAuc2V0U3R5bGVQcm9wZXJ0eSgiY29sb3IiLCAiYmx1ZSAhaW1wb3J0YW50Iik7Cg==