Closed Bug 616672 Opened 9 years ago Closed 8 years ago

Warn on access of LSProgressEvent.position, LSProgressEvent.totalSize


(Core :: DOM: Core & HTML, defect, P1)






(Reporter: Ms2ger, Assigned: Ms2ger)




(1 file, 3 obsolete files)

No description provided.
Attached patch Patch v1 (obsolete) — Splinter Review
Like this?
Attachment #495288 - Flags: review?(Olli.Pettay)
Target Milestone: --- → mozilla2.0
Comment on attachment 495288 [details] [diff] [review]
Patch v1

>+static void
>+ReportUseOfDeprecatedMethod(nsDOMProgressEvent* aPresContext, const char* aWarning)
Strangely named parameter.

>+  nsContentUtils::ReportToConsole(nsContentUtils::eDOM_PROPERTIES,
>+                                  aWarning,
>+                                  nsnull, 0,
>+                                  aInner->PresContext()->Document()->GetDocumentURI(),
>+                                  EmptyString(), 0, 0,
>+                                  nsIScriptError::warningFlag,
>+                                  "DOM Events");
Does this really work? How do you guarantee that PresContext() doesn't return null.
And I think we should report the warning only once per document.
(We do that for nsDocument::GetBoxObjectFor)
Perhaps you could use the target of the event to get the document?
Attachment #495288 - Flags: review?(Olli.Pettay) → review+
Attached patch Patch v2 (obsolete) — Splinter Review
I didn't even realize I already had a patch here...
Attachment #495288 - Attachment is obsolete: true
Attachment #561217 - Flags: review?(Olli.Pettay)
Comment on attachment 561217 [details] [diff] [review]
Patch v2

>+nsXMLHttpProgressEvent::WarnAboutLSProgressEvent(nsIDocument::DeprecatedOperations aOperation)
>+  nsPresContext* presContext = mInner->GetPresContext();
>+  if (!presContext) {
>+    return;
>+  }
Is presContext actually ever set?

I think it would be better to add something so that you could get
document from the target of the event.
(XHR's owner's document)
Attachment #561217 - Flags: review?(Olli.Pettay) → review-
Attached patch Patch v3 (obsolete) — Splinter Review
I found a Window... Does this look better?
Attachment #561217 - Attachment is obsolete: true
Attachment #561506 - Flags: review?(Olli.Pettay)
Target Milestone: mozilla2.0 → mozilla9
Attached patch Patch v3.1Splinter Review
And now a try that builds...
Attachment #561506 - Attachment is obsolete: true
Attachment #561547 - Flags: review?(Olli.Pettay)
Attachment #561506 - Flags: review?(Olli.Pettay)
Comment on attachment 561547 [details] [diff] [review]
Patch v3.1

>   // Use nsDOMProgressEvent so that we can forward
>   // most of the method calls easily.
>   nsRefPtr<nsDOMProgressEvent> mInner;
>+  nsCOMPtr<nsPIDOMWindow> mWindow;
You need to add this to cycle collection

>diff --git a/content/events/src/nsDOMEvent.h b/content/events/src/nsDOMEvent.h
>--- a/content/events/src/nsDOMEvent.h
>+++ b/content/events/src/nsDOMEvent.h
>@@ -218,16 +218,19 @@ public:
>   static PopupControlState GetEventPopupControlState(nsEvent *aEvent);
>   static void PopupAllowedEventsChanged();
>   static void Shutdown();
>   static const char* GetEventName(PRUint32 aEventType);
>+  nsPresContext* GetPresContext() { return mPresContext; }
Attachment #561547 - Flags: review?(Olli.Pettay) → review+
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [strings]
Target Milestone: mozilla9 → mozilla10
PositionWarning=Use XMLHttpRequest's progress events' position attribute is deprecated.
TotalSizeWarning=Use XMLHttpRequest's progress events' totalSize attribute is deprecated.

I suppose it should be "Use of" instead of "Use", but honestly I have big problems translating this string. Could you please try to rephrase it or give an equivalent sentence?
flod, what this means is that:
- XMLHttpRequest progress events have attributes called "position" and "totalSize".
- Use of these attributes is now deprecated and they will be removed at a later time.
- When a web developer uses these attributes, he'll get this warning telling them not to use these attributes.

To rephrase this for you:

'Using the "position'/'totalSize' attributes of XMLHttpRequest progress events is deprecated'

Hope this makes it clear for you. :)
(In reply to Marek Stępień :marcoos from comment #10)
> Hope this makes it clear for you. :)

Yes, a lot clearer. I already translated in this way, but now I'm sure that's right ;-)
Blocks: 695811
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.