Bug 158294 - Refactor showModalDialog handling in JSDOMWindowCustom
Summary: Refactor showModalDialog handling in JSDOMWindowCustom
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks: 158178
  Show dependency treegraph
 
Reported: 2016-06-01 23:43 PDT by Gavin Barraclough
Modified: 2016-06-03 00:09 PDT (History)
4 users (show)

See Also:


Attachments
Fix (3.75 KB, patch)
2016-06-02 23:36 PDT, Gavin Barraclough
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2016-06-01 23:43:00 PDT
The way this is currently implemented, for accessing the showModalDialog property there is effectively a duplication of the tail of the function modified to call Base::getOwnPropertySlot instead of getStaticPropertySlot. It does so based on the assumption that Base::getOwnPropertySlot is not going to search the static tables (containing the property we wish to omit).

However as a part of bug #158178 I plan to change it such that Base::getOwnPropertySlot does also search the static tables. Refactor this code to no longer depend on Base::getOwnPropertySlot bypassing the static tables. Always perform a lookup that will check both property storage & static tables. If the object does contain the property, check explicitly for the value we're intending to suppress.
Comment 1 Gavin Barraclough 2016-06-02 23:36:22 PDT
Created attachment 280428 [details]
Fix
Comment 2 WebKit Commit Bot 2016-06-02 23:38:42 PDT
Attachment 280428 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:58:  The parameter name "state" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:246:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gavin Barraclough 2016-06-03 00:09:01 PDT
Committed revision 201638.