Skip to content

Comments

FIX(client): resolve list tags, etc. polluting log#5619

Merged
Krzmbrzl merged 1 commit intomumble-voip:masterfrom
dexgs:chat-fix
Apr 9, 2022
Merged

FIX(client): resolve list tags, etc. polluting log#5619
Krzmbrzl merged 1 commit intomumble-voip:masterfrom
dexgs:chat-fix

Conversation

@dexgs
Copy link
Contributor

@dexgs dexgs commented Apr 8, 2022 •
edited
Loading

As reported in #4491, #4986 and #5430, since the changes to the chat in
the 1.4.x release, some tags would "pollute" the rest of the log, i.e.
cause future log entries to be contained within them.

The source of the issue seems to be that the insertBlock method of
QTextCursor appears to correspond to the <p> HTML tag. The <p> tag
may only contain inline elements, but QTextEdit will not fail outright
when inserting block-level elements and will instead attempt to correct
the invalid input which results in the behaviour reported in the above
issues (as far as I can tell).

My proposed solution is to use the insertFrame method for all
messages.

Fixes #4491

Checks

@dexgs
FIX(client): resolve list tags, etc. polluting log
ae097d6 
As reported in mumble-voip#4491, mumble-voip#4986 and mumble-voip#5430, since the changes to the chat in
the 1.4.x release, some tags would "pollute" the rest of the log, i.e.
cause future log entries to be contained within them.

The source of the issue seems to be that the `insertBlock` method of
`QTextCursor` appears to correspond to the `<p>` HTML tag. The `<p>` tag
may only contain inline elements, but `QTextEdit` will not fail outright
when inserting block-level elements and will instead attempt to correct
the invalid input which results in the behaviour reported in the above
issues (as far as I can tell).

My proposed solution is to use the `insertFrame` method for all
messages.

Fixes mumble-voip#4491
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Apr 8, 2022

Ah great! This bug was getting annoying 👀
I'll try to test this tomorrow or so to verify that everything is working as expected 👍

@Krzmbrzl Krzmbrzl added bug A bug (error) in the software client backport-needed labels Apr 8, 2022
Krzmbrzl
Krzmbrzl approved these changes Apr 9, 2022
View reviewed changes
Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work as expected 👍

@Krzmbrzl Krzmbrzl merged commit 0e321eb into mumble-voip:master Apr 9, 2022
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Apr 9, 2022

Thank you very much for fixing this ❤️

@Krzmbrzl Krzmbrzl mentioned this pull request Apr 9, 2022
Merged
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Apr 9, 2022

💚 All backports created successfully

Status Branch Result
✅ 1.4.x

Questions ?

Please refer to the Backport tool documentation

dexgs added a commit to dexgs/mumble that referenced this pull request Aug 22, 2022
@dexgs
FIX(ui): resolve text/icon scaling issues
fe71898 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Added minimum icon size for channel status icons which now scale since PR mumble-voip#5772
Initially, icons were too small at 100% scale.
@dexgs dexgs mentioned this pull request Aug 22, 2022
Merged
1 task
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 22, 2022
@dexgs
FIX(ui): resolve text/icon scaling issues
a813744 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Added minimum icon size for channel status icons which now scale since PR mumble-voip#5772
Initially, icons were too small at 100% scale.
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 22, 2022
@dexgs
FIX(ui): resolve text/icon scaling issues
369d963 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Added minimum icon size for channel status icons which now scale since PR mumble-voip#5772
Initially, icons were too small at 100% scale.
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 22, 2022
@dexgs
FIX(ui): resolve log text scaling issues
6d1dc60 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 22, 2022
@dexgs
FIX(ui): resolve log text scaling issues
1badd1f 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 25, 2022
@dexgs
FIX(client, ui): resolve log text scaling issues
a1fac4e 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.
dexgs added a commit to dexgs/mumble that referenced this pull request Aug 25, 2022
@dexgs
FIX(client, ui): resolve log text scaling issues
ece450a 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Fixes mumble-voip#5818
Krzmbrzl added a commit that referenced this pull request Aug 27, 2022
@Krzmbrzl
Merge PR #5820: FIX(client, ui): resolve log text scaling issues
6067c4a 
Fixed chat log scaling issue introduced by PR #5619
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Fixes #5818
Hartmnt pushed a commit to Hartmnt/mumble that referenced this pull request Sep 1, 2022
@dexgs @Hartmnt
FIX(client, ui): resolve log text scaling issues
ba49ad8 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Fixes mumble-voip#5818
Krzmbrzl pushed a commit to Krzmbrzl/mumble that referenced this pull request Sep 8, 2022
@dexgs @Krzmbrzl
FIX(client, ui): resolve log text scaling issues
15ac2c5 
Fixed chat log scaling issue introduced by PR mumble-voip#5619 (my fault, whoops)
The bug fixed by that commit is still fixed after this PR, but the
regression introduced has been resolved.

Fixes mumble-voip#5818

(cherry picked from commit ece450a)
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Feb 2, 2023

It seems like these changes may have caused a regression in selection behavior: #6045 (comment)

@dexgs
Copy link
Contributor Author

dexgs commented Feb 2, 2023

@Krzmbrzl, the changes I made in this PR have already been reverted to the best of my knowledge. The current fix for the issue targeted by this PR is #5927. I will still take a look and make sure the current issue isn't caused by this PR.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Feb 2, 2023 •
edited
Loading

Yes they are - this also wasn't a call to action from me to you but rather a note for future visitors. I just stumbled upon this during bisecting the linked issue and this might become relevant again once we try to fix the current bug :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Krzmbrzl Krzmbrzl Krzmbrzl approved these changes

Assignees

No one assigned

Labels

bug A bug (error) in the software client

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HTML generated by bots mixes with chat text on latest master

2 participants

@dexgs @Krzmbrzl