Summary: | [Qt] QScriptValue::strictlyEquals is broken | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jędrzej Nowacki <jedrzej.nowacki> | ||||||
Component: | JavaScriptCore | Assignee: | Jędrzej Nowacki <jedrzej.nowacki> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, jedrzej.nowacki, kenneth, kent.hansen, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 38987 | ||||||||
Bug Blocks: | 31863 | ||||||||
Attachments: |
|
Description
Jędrzej Nowacki
2010-03-25 07:44:25 PDT
The fix will be done in two steps; 1. reorganize the QScriptValue autotest suite (bug 38987) 2. fix the strictlyEquals method and add an autotest to cover it. Created attachment 55853 [details]
Fix v1
The patch depends on 38987 and will not be clearly applied unless 38987 will be accepted.
Comment on attachment 55853 [details]
Fix v1
I see code like
571 if (other->isJSBased()) {
572 assignEngine(other->engine());
573 return JSValueIsStrictEqual(context(), value(), other->value());
574 }
in two-three places, maybe make it an inline method? or will this only be used here?
(In reply to comment #3) > (From update of attachment 55853 [details]) > I see code like > > 571 if (other->isJSBased()) { > 572 assignEngine(other->engine()); > 573 return JSValueIsStrictEqual(context(), value(), other->value()); > 574 } > > in two-three places, maybe make it an inline method? or will this only be used here? Lets say only in 2,5 places, one is slightly different :-). The snippet will be used only in QScriptValuePrivate::strictlyEquals. Comment on attachment 55853 [details]
Fix v1
c- as 38987 has not been accepted yet.
Created attachment 57022 [details]
Fix v1.01
Patch was rebased against trunk (right now it should apply correctly).
Attachment 57022 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WARNING: File exempt from style guide. Skipping: "JavaScriptCore/qt/api/qscriptvalue_p.h"
WARNING: File exempt from style guide. Skipping: "JavaScriptCore/qt/api/qscriptvalue.cpp"
JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_comparison.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 6 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #7) > Attachment 57022 [details] did not pass style-queue: > If any of these errors are false positives, please file a bug against check-webkit-style. Bugs 35143, 35151 Comment on attachment 55853 [details] Fix v1 Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 55853 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 57022 [details] Fix v1.01 Clearing flags on attachment: 57022 Committed r60585: <http://trac.webkit.org/changeset/60585> All reviewed patches have been landed. Closing bug. |