Improving UTF-8 path detection and conversion (OpenShot#2525)

* Improving support for UTF-8 paths during the converstion from abs to rel (i.e. saving project)

* Removing strict JSON parsing and pretty printing OSP project file JSON

* Removing utf-8 parsing on read / write

* Manually escape backslashes (for windows paths and certain utf-8 strings)

* Experimental conversion from bytestring to unicode strings in Python (prior to any backslash replacing), and then re-encode the paths. This is to prevent breaking the JSON.

* Preventing the tutorial from popping up momentarily even when no tutorials are visible

* Adding debug info and more work on correctly parsing and replacing paths without breaking them

* Replacing utf parsing when converting paths to absolute

* Normalize path when searching for missing file paths (and combining folders and file names)

* Replacing backslashes with forward slashes for saving relative paths (due to crashes in Windows)

* Fixing invalid utf logic when converting to abs paths

* Switching to re.sub for regex substitutions, instead of slower str.replace (for large projects with hundreds or thousands of matches)

src/classes/json_data.py

@@ -40,7 +40,8 @@
 from classes import info
 
 # Compiled path regex
-path_regex = re.compile(r'\"(?:image|path)\":.*?\"(.*?)\"')
+path_regex = re.compile(r'\"(image|path)\":.*?\"(.*?)\"', re.UNICODE)
+path_context = {}
 
 
 class JsonDataStore:
@@ -134,7 +135,7 @@ def read_from_file(self, file_path, path_mode="ignore"):
                     if path_mode == "absolute":
                         # Convert any paths to absolute
                         contents = self.convert_paths_to_absolute(file_path, contents)
-                    return json.loads(contents)
+                    return json.loads(contents, strict=False)
         except Exception as ex:
             msg = ("Couldn't load {} file: {}".format(self.data_type, ex))
             log.error(msg)
@@ -146,7 +147,7 @@ def read_from_file(self, file_path, path_mode="ignore"):
     def write_to_file(self, file_path, data, path_mode="ignore", previous_path=None):
         """ Save JSON settings to a file """
         try:
-            contents = json.dumps(data)
+            contents = json.dumps(data, indent=4, sort_keys=True)
             if path_mode == "relative":
                 # Convert any paths to relative
                 contents = self.convert_paths_to_relative(file_path, previous_path, contents)
@@ -157,60 +158,89 @@ def write_to_file(self, file_path, data, path_mode="ignore", previous_path=None)
             log.error(msg)
             raise Exception(msg)
 
+    def replace_string_to_absolute(self, match):
+        """Replace matched string for converting paths to relative paths"""
+        key = match.groups(0)[0]
+        path = match.groups(0)[1]
+
+        # Find absolute path of file (if needed)
+        utf_path = json.loads('"%s"' % path, encoding="utf-8")  # parse bytestring into unicode string
+        if "@transitions" not in utf_path and not os.path.isabs(utf_path):
+            # Convert path to the correct relative path (based on the existing folder)
+            new_path = os.path.abspath(os.path.join(path_context.get("existing_project_folder", ""), utf_path))
+            new_path = json.dumps(new_path)  # Escape backslashes
+            return '"%s": %s' % (key, new_path)
+
+        # Determine if @transitions path is found
+        elif "@transitions" in path:
+            new_path = path.replace("@transitions", os.path.join(info.PATH, "transitions"))
+            new_path = json.dumps(new_path)  # Escape backslashes
+            return '"%s": %s' % (key, new_path)
+
     def convert_paths_to_absolute(self, file_path, data):
         """ Convert all paths to absolute using regex """
         try:
             # Get project folder
-            existing_project_folder = os.path.dirname(file_path)
-
-            # Find all "path" attributes in the JSON string
-            for path in path_regex.findall(data):
-                # Find absolute path of file (if needed)
-                if "@transitions" not in path and not os.path.isabs(path):
-                    # Convert path to the correct relative path (based on the existing folder)
-                    new_path = os.path.abspath(os.path.join(existing_project_folder, path))
-                    data = data.replace('"%s"' % path, '"%s"' % new_path)
+            path_context["new_project_folder"] = os.path.dirname(file_path)
+            path_context["existing_project_folder"] = os.path.dirname(file_path)
 
-                # Determine if @transitions path is found
-                elif "@transitions" in path:
-                    new_path = path.replace("@transitions", os.path.join(info.PATH, "transitions"))
-                    data = data.replace('"%s"' % path, '"%s"' % new_path)
+            # Optimized regex replacement
+            data = re.sub(path_regex, self.replace_string_to_absolute, data)
 
         except Exception as ex:
             log.error("Error while converting relative paths to absolute paths: %s" % str(ex))
 
         return data
 
+    def replace_string_to_relative(self, match):
+        """Replace matched string for converting paths to relative paths"""
+        key = match.groups(0)[0]
+        path = match.groups(0)[1]
+        utf_path = json.loads('"%s"' % path, encoding="utf-8")  # parse bytestring into unicode string
+        folder_path, file_path = os.path.split(os.path.abspath(utf_path))
+
+        # Determine if thumbnail path is found
+        if info.THUMBNAIL_PATH in folder_path:
+            # Convert path to relative thumbnail path
+            new_path = os.path.join("thumbnail", file_path).replace("\\", "/")
+            new_path = json.dumps(new_path)  # Escape backslashes
+            return '"%s": %s' % (key, new_path)
+
+        # Determine if @transitions path is found
+        elif os.path.join(info.PATH, "transitions") in folder_path:
+            # Yes, this is an OpenShot transitions
+            folder_path, category_path = os.path.split(folder_path)
+
+            # Convert path to @transitions/ path
+            new_path = os.path.join("@transitions", category_path, file_path).replace("\\", "/")
+            new_path = json.dumps(new_path)  # Escape backslashes
+            return '"%s": %s' % (key, new_path)
+
+        # Find absolute path of file (if needed)
+        else:
+            # Convert path to the correct relative path (based on the existing folder)
+            orig_abs_path = os.path.abspath(utf_path)
+
+            # Remove file from abs path
+            orig_abs_folder = os.path.split(orig_abs_path)[0]
+
+            # Calculate new relateive path
+            new_rel_path_folder = os.path.relpath(orig_abs_folder, path_context.get("new_project_folder", ""))
+            new_rel_path = os.path.join(new_rel_path_folder, file_path).replace("\\", "/")
+            new_rel_path = json.dumps(new_rel_path)  # Escape backslashes
+            return '"%s": %s' % (key, new_rel_path)
+
     def convert_paths_to_relative(self, file_path, previous_path, data):
         """ Convert all paths relative to this filepath """
         try:
             # Get project folder
-            new_project_folder = os.path.dirname(file_path)
-            existing_project_folder = os.path.dirname(file_path)
+            path_context["new_project_folder"] = os.path.dirname(file_path)
+            path_context["existing_project_folder"] = os.path.dirname(file_path)
             if previous_path:
-                existing_project_folder = os.path.dirname(previous_path)
-
-            # Find all "path" attributes in the JSON string
-            for path in path_regex.findall(data):
-                folder_path, file_path = os.path.split(path)
-
-                # Find absolute path of file (if needed)
-                if not os.path.join(info.PATH, "transitions") in folder_path:
-                    # Convert path to the correct relative path (based on the existing folder)
-                    orig_abs_path = path
-                    if not os.path.isabs(path):
-                        orig_abs_path = os.path.abspath(os.path.join(existing_project_folder, path))
-                    new_rel_path = os.path.relpath(orig_abs_path, new_project_folder)
-                    data = data.replace('"%s"' % path, '"%s"' % new_rel_path)
-
-                # Determine if @transitions path is found
-                else:
-                    # Yes, this is an OpenShot transitions
-                    folder_path, category_path = os.path.split(folder_path)
-
-                    # Convert path to @transitions/ path
-                    new_path = os.path.join("@transitions", category_path, file_path)
-                    data = data.replace('"%s"' % path, '"%s"' % new_path)
+                path_context["existing_project_folder"] = os.path.dirname(previous_path)
+
+            # Optimized regex replacement
+            data = re.sub(path_regex, self.replace_string_to_relative, data)
 
         except Exception as ex:
             log.error("Error while converting absolute paths to relative paths: %s" % str(ex))

src/classes/project_data.py

@@ -423,7 +423,7 @@ def read_legacy_project_file(self, file_path):
                         try:
                             clip = openshot.Clip(item.name)
                             reader = clip.Reader()
-                            file_data = json.loads(reader.Json())
+                            file_data = json.loads(reader.Json(), strict=False)
 
                             # Determine media type
                             if file_data["has_video"] and not self.is_image(file_data):
@@ -492,7 +492,7 @@ def read_legacy_project_file(self, file_path):
                             c = openshot.Clip(file_path)
 
                             # Append missing attributes to Clip JSON
-                            new_clip = json.loads(c.Json())
+                            new_clip = json.loads(c.Json(), strict=False)
                             new_clip["file_id"] = file.id
                             new_clip["title"] = filename
                             new_clip["image"] = thumb_path
@@ -511,19 +511,19 @@ def read_legacy_project_file(self, file_path):
                             if clip.video_fade_in:
                                 # Add keyframes
                                 start = openshot.Point(round(clip.start_time * fps_float) + 1, 0.0, openshot.BEZIER)
-                                start_object = json.loads(start.Json())
+                                start_object = json.loads(start.Json(), strict=False)
                                 end = openshot.Point(round((clip.start_time + clip.video_fade_in_amount) * fps_float) + 1, 1.0, openshot.BEZIER)
-                                end_object = json.loads(end.Json())
+                                end_object = json.loads(end.Json(), strict=False)
                                 new_clip["alpha"]["Points"].append(start_object)
                                 new_clip["alpha"]["Points"].append(end_object)
 
                             # Video Fade OUT
                             if clip.video_fade_out:
                                 # Add keyframes
                                 start = openshot.Point(round((clip.end_time - clip.video_fade_out_amount) * fps_float) + 1, 1.0, openshot.BEZIER)
-                                start_object = json.loads(start.Json())
+                                start_object = json.loads(start.Json(), strict=False)
                                 end = openshot.Point(round(clip.end_time * fps_float) + 1, 0.0, openshot.BEZIER)
-                                end_object = json.loads(end.Json())
+                                end_object = json.loads(end.Json(), strict=False)
                                 new_clip["alpha"]["Points"].append(start_object)
                                 new_clip["alpha"]["Points"].append(end_object)
 
@@ -532,26 +532,26 @@ def read_legacy_project_file(self, file_path):
                                 new_clip["volume"]["Points"] = []
                             else:
                                 p = openshot.Point(1, clip.volume / 100.0, openshot.BEZIER)
-                                p_object = json.loads(p.Json())
+                                p_object = json.loads(p.Json(), strict=False)
                                 new_clip["volume"] = { "Points" : [p_object]}
 
                             # Audio Fade IN
                             if clip.audio_fade_in:
                                 # Add keyframes
                                 start = openshot.Point(round(clip.start_time * fps_float) + 1, 0.0, openshot.BEZIER)
-                                start_object = json.loads(start.Json())
+                                start_object = json.loads(start.Json(), strict=False)
                                 end = openshot.Point(round((clip.start_time + clip.video_fade_in_amount) * fps_float) + 1, clip.volume / 100.0, openshot.BEZIER)
-                                end_object = json.loads(end.Json())
+                                end_object = json.loads(end.Json(), strict=False)
                                 new_clip["volume"]["Points"].append(start_object)
                                 new_clip["volume"]["Points"].append(end_object)
 
                             # Audio Fade OUT
                             if clip.audio_fade_out:
                                 # Add keyframes
                                 start = openshot.Point(round((clip.end_time - clip.video_fade_out_amount) * fps_float) + 1, clip.volume / 100.0, openshot.BEZIER)
-                                start_object = json.loads(start.Json())
+                                start_object = json.loads(start.Json(), strict=False)
                                 end = openshot.Point(round(clip.end_time * fps_float) + 1, 0.0, openshot.BEZIER)
-                                end_object = json.loads(end.Json())
+                                end_object = json.loads(end.Json(), strict=False)
                                 new_clip["volume"]["Points"].append(start_object)
                                 new_clip["volume"]["Points"].append(end_object)
 
@@ -589,9 +589,9 @@ def read_legacy_project_file(self, file_path):
                                 "position": trans.position_on_track,
                                 "start": 0,
                                 "end": trans.length,
-                                "brightness": json.loads(brightness.Json()),
-                                "contrast": json.loads(contrast.Json()),
-                                "reader": json.loads(transition_reader.Json()),
+                                "brightness": json.loads(brightness.Json(), strict=False),
+                                "contrast": json.loads(contrast.Json(), strict=False),
+                                "reader": json.loads(transition_reader.Json(), strict=False),
                                 "replace_image": False
                             }
 
@@ -845,7 +845,7 @@ def check_if_paths_are_valid(self):
                 # try to find file with previous starting folder:
                 if starting_folder and os.path.exists(os.path.join(starting_folder, file_name_with_ext)):
                     # Update file path
-                    path = os.path.join(starting_folder, file_name_with_ext)
+                    path = os.path.abspath(os.path.join(starting_folder, file_name_with_ext))
                     file["path"] = path
                     get_app().updates.update(["import_path"], os.path.dirname(path))
                     log.info("Auto-updated missing file: %s" % path)
@@ -857,7 +857,7 @@ def check_if_paths_are_valid(self):
                     log.info("Missing folder chosen by user: %s" % starting_folder)
                     if starting_folder:
                         # Update file path and import_path
-                        path = os.path.join(starting_folder, file_name_with_ext)
+                        path = os.path.abspath(os.path.join(starting_folder, file_name_with_ext))
                         file["path"] = path
                         get_app().updates.update(["import_path"], os.path.dirname(path))
                     else:
@@ -876,7 +876,7 @@ def check_if_paths_are_valid(self):
                 # try to find clip with previous starting folder:
                 if starting_folder and os.path.exists(os.path.join(starting_folder, file_name_with_ext)):
                     # Update clip path
-                    path = os.path.join(starting_folder, file_name_with_ext)
+                    path = os.path.abspath(os.path.join(starting_folder, file_name_with_ext))
                     clip["reader"]["path"] = path
                     log.info("Auto-updated missing file: %s" % clip["reader"]["path"])
                     break
@@ -886,7 +886,7 @@ def check_if_paths_are_valid(self):
                     log.info("Missing folder chosen by user: %s" % starting_folder)
                     if starting_folder:
                         # Update clip path
-                        path = os.path.join(starting_folder, file_name_with_ext)
+                        path = os.path.abspath(os.path.join(starting_folder, file_name_with_ext))
                         clip["reader"]["path"] = path
                     else:
                         log.info('Removed missing clip: %s' % file_name_with_ext)

src/classes/updates.py

@@ -105,7 +105,7 @@ def load_json(self, value):
         """ Load this UpdateAction from a JSON string """
 
         # Load JSON string
-        update_action_dict = json.loads(value)
+        update_action_dict = json.loads(value, strict=False)
 
         # Set the Update Action properties
         self.type = update_action_dict.get("type")
@@ -176,13 +176,13 @@ def save_history(self, project, history_length):
         history_length_int = int(history_length)
         for action in self.redoHistory[-history_length_int:]:
             if action.type != "load" and action.key[0] != "history":
-                actionDict = json.loads(action.json())
+                actionDict = json.loads(action.json(), strict=False)
                 redo_list.append(actionDict)
             else:
                 log.info("Saving redo history, skipped key: %s" % str(action.key))
         for action in self.actionHistory[-history_length_int:]:
             if action.type != "load" and action.key[0] != "history":
-                actionDict = json.loads(action.json())
+                actionDict = json.loads(action.json(), strict=False)
                 undo_list.append(actionDict)
             else:
                 log.info("Saving undo, skipped key: %s" % str(action.key))

src/windows/ui/main-window.ui

@@ -379,7 +379,7 @@
     <enum>Qt::NoContextMenu</enum>
    </property>
    <property name="visible">
-    <bool>true</bool>
+    <bool>false</bool>
    </property>
    <property name="autoFillBackground">
     <bool>false</bool>
@@ -416,7 +416,7 @@
      <enum>Qt::NoContextMenu</enum>
     </property>
     <property name="visible">
-     <bool>true</bool>
+     <bool>false</bool>
     </property>
     <property name="autoFillBackground">
      <bool>false</bool>