Closed Bug 1132679 Opened 5 years ago Closed 5 years ago

Enable fail on warnings on windows widget

Categories

(Core :: Widget: Win32, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Fix remaining build warnings in widget/windows and enable FAIL_ON_WARNINGS on that dir.
Attached patch patchSplinter Review
Assignee: nobody → quanxunzhen
Attachment #8563660 - Flags: review?(jmathies)
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?
(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.
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 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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.