Summary: | [iOS] iPad: Occasional <select> crashes attempting to scroll to non-existing row 0 in viewWillAppear | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | WebKit2 | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | buildbot, ddkilzer, enrica, joepeck, rniwa | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-10-28 18:15:17 PDT
Created attachment 240586 [details]
[PATCH] Proposed Fix
Tested with many different <select>s. This still properly scrolls to the selected option in a <select> when displaying the table. I was unable to encounter a case that would have caused the original exception/crash though. Auditing the code I am not sure how it would be possible either.
Comment on attachment 240586 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240586&action=review r=me. Did you try using JavaScript to mutate the <select> element when trying to change it? We now fire on change events, right? Do we allow JavaScript to run while the wheel-of-time is up? > Source/WebKit2/UIProcess/ios/forms/WKFormSelectPopover.mm:147 > + if (_singleSelectionIndex != NSNotFound && _singleSelectionSection < (NSUInteger)[self.tableView numberOfSections] && _singleSelectionIndex < (NSUInteger)[self.tableView numberOfRowsInSection:_singleSelectionSection]) { This would read better if new test conditions were pulled out into their own descriptively named BOOL variables. (In reply to comment #3) > Comment on attachment 240586 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240586&action=review > > r=me. Did you try using JavaScript to mutate the <select> element when > trying to change it? We now fire on change events, right? Do we allow > JavaScript to run while the wheel-of-time is up? I did test this as much as I could. The UI gets the list once, never updates it (even if JavaScript modifies the <select> in the background) and this scroll-to within the select is only within the list of items we've already gotten. > > Source/WebKit2/UIProcess/ios/forms/WKFormSelectPopover.mm:147 > > + if (_singleSelectionIndex != NSNotFound && _singleSelectionSection < (NSUInteger)[self.tableView numberOfSections] && _singleSelectionIndex < (NSUInteger)[self.tableView numberOfRowsInSection:_singleSelectionSection]) { > > This would read better if new test conditions were pulled out into their own > descriptively named BOOL variables. Each condition builds off of the previous condition. I don't think we should run any of the later ones if an earlier one failed. I may restructure with early returns then. |