Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

CB 13855 Fix Windows 8.1 crash on cordova-windows@5 #247

Merged
merged 4 commits into from Feb 8, 2018

Conversation

brodybits
Copy link

@brodybits brodybits commented Feb 8, 2018

Platforms affected

Windows

What does this PR do?

CB-13855 (https://issues.apache.org/jira/browse/CB-13855): Fix crash of 8.1-win (Windows 8.1) build on cordova-windows@5 in 5.0.x branch

What testing has been done on this change?

Reproduction: Crash was consistently observed on Windows 8.1 build with cordova-windows@5.0.0 (on my Windows 10 development system).

Verification of bug fix: If I would create a test project (cordova create CB-13855-test2), add the Windows platform using the following command:

 cordova platform add https://github.com/brodybits/cordova-windows#CB-13855

and then run Windows 8.1 x86 or x64 Release build (or Windows 8.1 Debug build) on my Windows 10 development system then the app would start without crashing.

Sanity testing: if I would run Windows Phone 8.1 ARM Debug or Release build on my Windows Mobile (Windows 10 Mobile) test device the app would start up as normal. If I would run Windows 10 x86 or x64 Release Build on my Windows 10 development system, or run Windows 10 ARM Release build on my Windows 10 Mobile device, the app would start up as normal.

Checklist

  • Reported an issue in the JIRA database (https://issues.apache.org/jira/browse/CB-13855)
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. (almost)
    Added automated test coverage as appropriate for this change. not done for this change

Additional notes

I am raising this fix out of general frustration with cordova-windows@5 which would consistently crash on Windows 8.1 build. This issue was fixed on the master branch after multiple tries (PR #232 for CB-12784 (https://issues.apache.org/jira/browse/CB-12784) and PR #239 for CB-13175). I had expected a 5.1.0 release to be issued 1-2 weeks ago but just learned that the master branch is now targeted for cordova-windows@6.0.0 (http://callback.markmail.org/thread/rzfa2zoydyjq6xso) and cannot be released until a test failure is resolved (#246 (comment)).

I am raising this fix with the following commits:

I would definitely understand this to be a very unusual way to issue a bug fix. I manually applied the change to template/www/cordova.js since I could not understand how to automate this change in the 5.0.1 branch (not sure if this is possible or not). I will also understand if the cordova-windows team does not want me to mark the proposed 5.0.1 release.

I would be happy to rework this fix if needed to get the new cordova-windows@5 release not crashing on Windows 8.1 build. It has been broken for way too long.

@codecov-io
Copy link

Codecov Report

Merging #247 into 5.0.x will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            5.0.x     #247   +/-   ##
=======================================
  Coverage   76.91%   76.91%           
=======================================
  Files          16       16           
  Lines        2222     2222           
  Branches      439      439           
=======================================
  Hits         1709     1709           
  Misses        513      513

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7c99c8...f77f903. Read the comment docs.

@purplecabbage purplecabbage merged commit 7df7c1f into apache:5.0.x Feb 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants