Closed Bug 1006379 Opened 7 years ago Closed 7 years ago

JS error in language detection code when closing a tab that's still loading

Categories

(Firefox :: Translation, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: florian, Assigned: Gijs)

Details

(Whiteboard: [translation] p=3 s=it-32c-31a-30b.2 [qa-])

Attachments

(2 files, 1 obsolete file)

[Exception... "[JavaScript Error: "content is null" {file: "chrome://browser/content/content.js" line: 413}]'[JavaScript Error: "content is null" {file: "chrome://browser/content/content.js" line: 413}]' when calling method: [nsIWebProgressListener::onStateChange]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://browser/content/tabbrowser.xml :: _endRemoveTab :: line 2071"  data: yes]

Line 413 is:
    encoder.init(content.document, "text/plain", encoder.SkipInvisibleContent);

I noticed this JS error in my terminal when I closed a tab that was still loading.

I guess it would be trivial to just add a null check on "content" and return early if it's null, but looking at the values of the parameters given to the onStateChange method may be a better solution.
Mass move of translation bugs to the new Translation component.
Component: General → Translation
Version: Trunk → unspecified
Flags: firefox-backlog+
Whiteboard: [translation] → [translation] p=0
Whiteboard: [translation] p=0 → [translation] p=3
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Whiteboard: [translation] p=3 → [translation] p=3 s=it-32c-31a-30b.2 [qa?]
Whiteboard: [translation] p=3 s=it-32c-31a-30b.2 [qa?] → [translation] p=3 s=it-32c-31a-30b.2 [qa-]
AFAICT the only information we have is if the load aborted. So we can do this, but it'd break translation of pages where the user hits stop/escape or aborts the load for some other reason.
Attachment #8423099 - Flags: review?(felipc)
Ergh, turns out Cr isn't defined yet. Fixing that.
Attachment #8423100 - Flags: review?(felipc)
Attachment #8423099 - Attachment is obsolete: true
Attachment #8423099 - Flags: review?(felipc)
Comment on attachment 8423100 [details] [diff] [review]
don't attempt language detection on aborted page loads,

This attachment will conflict with attachment 8420802 [details] [diff] [review].
Alternatively, we could just nullcheck.
Attachment #8423123 - Flags: review?(felipc)
Attachment #8423100 - Flags: review?(felipc)
Comment on attachment 8423123 [details] [diff] [review]
don't attempt language detection if the document is gone,

Review of attachment 8423123 [details] [diff] [review]:
-----------------------------------------------------------------

I think detecting the language when the user has pressed Stop is a valid use case, so let's go with this option..

Do we get the same NS_BINDING_ABORTED for both a user-triggered stop and for a document-is-gone stop?
Attachment #8423123 - Flags: review?(felipc) → review+
(In reply to :Felipe Gomes from comment #6)
> Comment on attachment 8423123 [details] [diff] [review]
> don't attempt language detection if the document is gone,
> 
> Review of attachment 8423123 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think detecting the language when the user has pressed Stop is a valid use
> case, so let's go with this option..
> 
> Do we get the same NS_BINDING_ABORTED for both a user-triggered stop and for
> a document-is-gone stop?

Yes, AFAICT there's no way to distinguish the two cases from the web progress listener's pov. :-\

(I did also test and the other patch does indeed break the "press stop during page load, then translate" usecase)
remote:   https://hg.mozilla.org/integration/fx-team/rev/d2214e4edf1b
Whiteboard: [translation] p=3 s=it-32c-31a-30b.2 [qa-] → [translation][fixed-in-fx-team] p=3 s=it-32c-31a-30b.2 [qa-]
https://hg.mozilla.org/mozilla-central/rev/d2214e4edf1b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [translation][fixed-in-fx-team] p=3 s=it-32c-31a-30b.2 [qa-] → [translation] p=3 s=it-32c-31a-30b.2 [qa-]
Target Milestone: --- → Firefox 32
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.