Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit accessible-name-change signal for icons only when icons are renamed #979

Merged
merged 1 commit into from Apr 24, 2018
Merged

Conversation

joanmarie
Copy link
Contributor

caja_icon_canvas_item_set_property was emitting accessible-name-change
signals whenever the PROP_EDITABLE_TEXT was being set for an icon. This
method is called by caja_icon_container_request_update_all when the icon
view is first loaded. Thus irrelevant events are being emitted (the name
of each icon didn't change, as far as the user is concerned). Furthermore,
this floods assistive technologies any time the user opens a folder with
a large number of files.

Moving the notification out of caja_icon_canvas_item_set_property and
into end_renaming_mode (where ICON_TEXT_CHANGED is also emitted) solves
the problem of floods of irrelevant notifications while still emitting
the signal when the name actually changes.

caja_icon_canvas_item_set_property was emitting accessible-name-change
signals whenever the PROP_EDITABLE_TEXT was being set for an icon. This
method is called by caja_icon_container_request_update_all when the icon
view is first loaded. Thus irrelevant events are being emitted (the name
of each icon didn't change, as far as the user is concerned). Furthermore,
this floods assistive technologies any time the user opens a folder with
a large number of files.

Moving the notification out of caja_icon_canvas_item_set_property and
into end_renaming_mode (where ICON_TEXT_CHANGED is also emitted) solves
the problem of floods of irrelevant notifications while still emitting
the signal when the name actually changes.
Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

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

While I not sure what these signals do, this PR meant the difference between 2,466 and 106 entries in accerciser from object:property-change:accessible-name on opening /usr/bin in Caja-and the difference in whether Pluma became unresponsive when trying to paste them all in to count them.

@alexarnaud
Copy link
Member

Thank you @joanmarie for the fix. Do you think we could do the same for the list view?

Best regards,
Alex.

@monsta monsta requested a review from a team April 21, 2018 11:59
@joanmarie
Copy link
Contributor Author

@alexarnaud I am not seeing that event spam for the list view. Are you?

@raveit65
Copy link
Member

Honestly i don't see any different with or without both PRs in accerciser app.
But i am complete clueless to use this app.
Also, this app produces abrt alams about python3 in fedora 26 and causes a high proc load.
So i stop testing this.
Building works fine and i see no new build warnings.
So, if it works for you.....

Copy link
Contributor

@monsta monsta left a comment

Choose a reason for hiding this comment

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

Looks like this only affects a11y features, and I don't use them, so browsing and renaming in icon view works as before for me.

@lukefromdc lukefromdc requested a review from a team April 22, 2018 18:33
Copy link
Member

@flexiondotorg flexiondotorg left a comment

Choose a reason for hiding this comment

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

No regressions observed. Patch committed to Debian and patched package upload to Ubuntu 18.04.

@raveit65 raveit65 merged commit 3c3f62d into mate-desktop:master Apr 24, 2018
@alexarnaud
Copy link
Member

alexarnaud commented Apr 24, 2018 via email

@joanmarie
Copy link
Contributor Author

@alexarnaud I am having no delay problems using Orca on the list view. Are you?

@alexarnaud
Copy link
Member

@joanmarie How many files do you have in your test folder? Are you testing Caja or Nautilus ?

Best regards,
Alex.

@joanmarie
Copy link
Contributor Author

@alexarnaud: 10,000 and both caja and nautilus.

@Texou-
Copy link

Texou- commented Apr 24, 2018

@joanmarie Well here with Caja 1.20 without your patch I test to open a folder with 800 files and dir, and the delay is about 10 seconds before being loaded, with List View. Shorter with Compact view.

Regards,

@alexarnaud
Copy link
Member

@joanmarie Should we use the Orca patch to makes it fast or is it already fast for you without the orca patch?

Best regards,
Alex.

@alexarnaud
Copy link
Member

@joanmarie I've done a test, on my Debian Sid VM, with latest Orca from master and Caja 1.20 from unstable, I've observed a lag of 5 seconds on a folder of 10 000 files. I'm using a hard drive, not a SSD.

Best regards,
Alex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants