Enable fail on warnings on windows widget

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
x86_64
Windows 8.1
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Fix remaining build warnings in widget/windows and enable FAIL_ON_WARNINGS on that dir.
(Assignee)

Comment 1

4 years ago
Created attachment 8563660 [details] [diff] [review]
patch
Assignee: nobody → quanxunzhen
Attachment #8563660 - Flags: review?(jmathies)

Comment 3

4 years ago
Comment on attachment 8563660 [details] [diff] [review]
patch

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

::: widget/windows/nsFilePicker.cpp
@@ +639,5 @@
>  
> +/* static */ bool
> +nsFilePicker::GetFileNameWrapper(OPENFILENAMEW* ofn, PickerType aType)
> +{
> +  MOZ_SEH_TRY {

I think we can just get rid of this trap. If we remove that, can we put this back down in FilePickerWrapper?
(Assignee)

Comment 4

4 years ago
(In reply to Jim Mathies [:jimm] from comment #3)
> Comment on attachment 8563660 [details] [diff] [review]
> patch
> 
> Review of attachment 8563660 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/nsFilePicker.cpp
> @@ +639,5 @@
> >  
> > +/* static */ bool
> > +nsFilePicker::GetFileNameWrapper(OPENFILENAMEW* ofn, PickerType aType)
> > +{
> > +  MOZ_SEH_TRY {
> 
> I think we can just get rid of this trap. If we remove that, can we put this
> back down in FilePickerWrapper?

Yes, I think so.
(Assignee)

Comment 5

4 years ago
But I wonder whether we can get rid of the trap, given there are some discussion in the API page mentioned that it could raise exception. [1]

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms646927%28v=vs.85%29.aspx

Comment 6

4 years ago
Comment on attachment 8563660 [details] [diff] [review]
patch

yeah you're right. There are a few mentions of exception problems out there, none very clear on what happens. I guess we'll have to keep this.
Attachment #8563660 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/mozilla-central/rev/7d7c3210ea78
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.