Bug 115969
| Summary: | Value of FrameLoadTypeSame is suspicious | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
| Component: | Page Loading | Assignee: | 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)
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 | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Simon Fraser (smfr)
Or is FrameLoadTypeSame supposed to match FrameLoadTypeReplace? If so, that's a very fragile way to do it.
Alexey Proskuryakov
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)
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,