nsIWebNavigation::Reload() using mask flag crashes embedding app

RESOLVED FIXED in Future

Status

()

RESOLVED FIXED
18 years ago
16 years ago

People

(Reporter: depman1, Assigned: adamlock)

Tracking

Trunk
Future
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

18 years ago
reproduced in testembed. /mozilla/embedding/qa/testembed. Using 6/19 embedding
build.
1. Load a uri with the mask flag: 

rv = qaWebNav->LoadURI(NS_ConvertASCIItoUCS2(theUrl).GetUnicode(), 
			nsIWebNavigation::LOAD_FLAGS_MASK);

2. Do a Reload with the mask flag:

   rv = qaWebNav->Reload(LOAD_FLAGS_MASK);

Result: Get assertion: Reload command not updated to load flags! '((aReloadFlags
& 0xf0 == 0)', file ../mozilla/docshell/base/nsDocShell.cpp  line 2169. Crash.

In the debugger, get this call stack:
nsDebug:Assertion()               nsDebug.cpp
nsDocShell::Reload()              nsDocShell.cpp
nsWebBrowser::Reload()            nsDocShell.cpp

Expected: Should reload the previous uri fine.
(Assignee)

Comment 1

18 years ago
Quite a few flag combinations may crash embedding. We should put some logic 
into LoadURI and Reload that stops duff combinations or at least asserts on 
them.

The valid combinations are defined in this enum below:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.h#87
(Assignee)

Comment 2

18 years ago
Created attachment 39920 [details] [diff] [review]
Adds flag checks to LoadURI & Reload
(Assignee)

Comment 3

18 years ago
David could you apply the patch and see if it catches the strange flag 
combinations?

Potentially the Reload method could be even stricter, rejecting LoadURI 
specific flags.
(Reporter)

Comment 4

18 years ago
Adam, this patch did the trick (using yesterday's build). No crash for reloading
with mask flag.
(Assignee)

Comment 5

17 years ago
Rick is this bug still relevant?

Comment 6

17 years ago
It looks like this was just a debug ASSERT not an actual crash...
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla0.9.6
(Assignee)

Comment 7

17 years ago
Moving out
Target Milestone: mozilla0.9.6 → mozilla1.0

Updated

17 years ago
QA Contact: mdunn → depstein

Comment 8

17 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 9

16 years ago
Changing target milestone to 'Future' since 'mozilla1.0.1' came and went already.
Target Milestone: mozilla1.0.1 → Future
(Reporter)

Updated

16 years ago
QA Contact: depstein → carosendahl
(Assignee)

Comment 10

16 years ago
Created attachment 123831 [details] [diff] [review]
Patch 2

The bug that time forgot! I've updated the patch and run through various link
clicks, normal loads, history loads, reloads and charset reloads and it appears
to function fine.
Attachment #39920 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
Comment on attachment 123831 [details] [diff] [review]
Patch 2

Requesting an r/sr on this patch. It asserts and blocks illegal loadtype
combinations.
Attachment #123831 - Flags: superreview?(blizzard)
Attachment #123831 - Flags: review?(radha)
Comment on attachment 123831 [details] [diff] [review]
Patch 2

sr=blizzard
Attachment #123831 - Flags: superreview?(blizzard) → superreview+
Attachment #123831 - Flags: review?(radha) → review+
(Assignee)

Comment 13

16 years ago
Fix is checked into trunk
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.