online icon indicating copy to clipboard operation
online copied to clipboard
verified publisher icon CollaboraOnline

Revert "Use a Map type for TileManager.tiles"

Open jazevedo-coll opened this issue 7 months ago • 7 comments

This reverts commit 4a6eb1118d3bb3b25c9aeea548cd4c7bbfc9106e.

It was causing issues with font color in sessions with
multiple users and one of the users switches to
Dark mode.
  • Resolves: #
  • Target version: master

Summary

TODO

  • [ ] ...

Checklist

  • [ ] I have run make prettier-write and formatted the code.
  • [ ] All commits have Change-Id
  • [] I have run tests with make check
  • [x] I have issued make run and manually verified that everything looks okay
  • [x] Documentation (manuals or wiki) has been updated or is not required

CC: @eszkadev @Cwiiis

jazevedo-coll avatar May 15 '25 13:05 jazevedo-coll

This should not have changed behaviour, is this definitely the patch that caused it, and do we know why?

Cwiiis avatar May 15 '25 13:05 Cwiiis

This should not have changed behaviour, is this definitely the patch that caused it, and do we know why?

I do not know why. I reverted the patch locally, and tested it, and I could not reproduce the issue with this patch reverted.

jazevedo-coll avatar May 15 '25 13:05 jazevedo-coll

This should not have changed behaviour, is this definitely the patch that caused it, and do we know why?

I do not know why. I reverted the patch locally, and tested it, and I could not reproduce the issue with this patch reverted.

Have you since tried to reproduce the issue without the revert? I'm wondering if it's an intermittent issue - I'd have expected if there was an error in this patch that it would manifest differently.

Cwiiis avatar May 15 '25 13:05 Cwiiis

Have you since tried to reproduce the issue without the revert?

Yes, in today's COOL Weekly.

COOLWSD version: 25.04.2.1(git hash: 0968141f2c (E)) LOKit version: Collabora Office 25.04.2.1(git hash: 362b544a13)

Screenshot From 2025-05-15 11-56-15

jazevedo-coll avatar May 15 '25 13:05 jazevedo-coll

Lets revert for now - shame; but I expect the real problem is with accumulating tile deltas. I expect there is a race here.

mmeeks avatar May 15 '25 13:05 mmeeks

Remember Map and Object have some important differences in their behaviour. Like https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#objects_vs._maps

hfiguiere avatar May 15 '25 13:05 hfiguiere

Just to update on the status of this PR:

The commit that I proposed to be reverted in this PR, only seems to expose another underlying issue that has yet to be pinpointed. It would be preferable to discover the underlying issue and fix that.

jazevedo-coll avatar Jun 12 '25 10:06 jazevedo-coll

this pr is obsolete and the issue was solved by this other PR: https://github.com/CollaboraOnline/online/pull/12342

jazevedo-coll avatar Aug 14 '25 10:08 jazevedo-coll