Summary: | [Chromium] History related tests REGRESSED after r70723 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Naganov <mnaganov> | ||||
Component: | History | Assignee: | Mihai Parparita <mihaip> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, eric, fishd, mihaip | ||||
Priority: | P1 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Mikhail Naganov
2010-10-28 04:57:15 PDT
I’ll try to find some time to look at this today, but if someone else can fix it before I get to it, that would be great. I have limited experience with Chrome’s back/forward implementation. I'll take a stab at this. On guess is that this has something to do with the backItem, currentItem, and forwardItem functions. Perhaps itemAtIndex doesn’t give the same result or there was some side effect from the old version of the functions. Very little else changed in WebCore. (In reply to comment #3) > On guess is that this has something to do with the backItem, currentItem, and forwardItem functions. Perhaps itemAtIndex doesn’t give the same result or there was some side effect from the old version of the functions. Yes, BackForwardListChromium has a custom implementation of currentItem(). Either changing it back to virtual in BackForwardList.h or making itemAtIndex(0) in the Chromium version be the same as currentItem should fix things. (In reply to comment #4) > making itemAtIndex(0) in the Chromium version be the same as currentItem should fix things. That’s what we should do. In the future we probably don’t want a separate currentItem function. Unless we get rid of itemAtIndex altogether, we want to make sure that itemAtIndex(0) works properly. Created attachment 72249 [details]
Patch
(In reply to comment #5) > That’s what we should do. In the future we probably don’t want a separate currentItem function. Unless we get rid of itemAtIndex altogether, we want to make sure that itemAtIndex(0) works properly. Makes sense. Uploaded patch does that. Comment on attachment 72249 [details]
Patch
Looks good. Thanks!
Comment on attachment 72249 [details] Patch Clearing flags on attachment: 72249 Committed r70823: <http://trac.webkit.org/changeset/70823> All reviewed patches have been landed. Closing bug. |