Bug 115969

Summary: Value of FrameLoadTypeSame is suspicious
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, beidson, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Simon Fraser (smfr)
Reported 2013-05-11 14:50:00 PDT
This seems wrong: enum FrameLoadType { FrameLoadTypeStandard, FrameLoadTypeBack, FrameLoadTypeForward, FrameLoadTypeIndexedBackForward, // a multi-item hop in the backforward list FrameLoadTypeReload, // Skipped value: 'FrameLoadTypeReloadAllowingStaleData', still present in mac/win public API. Ready to be reused FrameLoadTypeSame = FrameLoadTypeReload + 2, // user loads same URL again (but not reload button) FrameLoadTypeRedirectWithLockedBackForwardList, // FIXME: Merge "lockBackForwardList", "lockHistory", "quickRedirect" and "clientRedirect" into a single concept of redirect. FrameLoadTypeReplace, FrameLoadTypeReloadFromOrigin, }; FrameLoadTypeSame = FrameLoadTypeReload + 2, // user loads same URL again (but not reload button) so FrameLoadTypeSame == FrameLoadTypeRedirectWithLockedBackForwardList ? And isn't FrameLoadTypeSame supposed to match WebFrameLoadTypeSame in WebKit?
Attachments
Simon Fraser (smfr)
Comment 1 2013-05-11 14:51:42 PDT
Or is FrameLoadTypeSame supposed to match FrameLoadTypeReplace? If so, that's a very fragile way to do it.
Alexey Proskuryakov
Comment 2 2013-05-12 21:34:04 PDT
Per this definition, FrameLoadTypeReplace is FrameLoadTypeReload + 4. In other words, enum FrameLoadType { FrameLoadTypeStandard, // 0 FrameLoadTypeBack, // 1 FrameLoadTypeForward, // 2 FrameLoadTypeIndexedBackForward, // 3 FrameLoadTypeReload, // 4 // Skipped value: 'FrameLoadTypeReloadAllowingStaleData', which used to be 5 FrameLoadTypeSame = FrameLoadTypeReload + 2, // 6 FrameLoadTypeRedirectWithLockedBackForwardList, // 7 FrameLoadTypeReplace, // 8 FrameLoadTypeReloadFromOrigin, //9 }; Can you clarify what seems wrong? AFAICT, it's exactly what the comment says - we removed FrameLoadTypeReloadAllowingStaleData without changing values of other values.
Simon Fraser (smfr)
Comment 3 2013-05-13 10:57:52 PDT
Tired brain was tired. Still, I think it would be clearer to replace: // Skipped value: 'FrameLoadTypeReloadAllowingStaleData', still present in mac/win public API. Ready to be reused FrameLoadTypeSame = FrameLoadTypeReload + 2, // user loads same URL again (but not reload button) with FrameLoadTypeUnused, // Skipped value: 'FrameLoadTypeReloadAllowingStaleData', still present in mac/win public API. Ready to be reused FrameLoadTypeSame,
Note You need to log in before you can comment on or make changes to this bug.