The context menu makes bad use of (kinda?) checkboxes

[memeplex]

The context menu shows three options (check word meaning, annotate, etc) and at the left side of each one there is a box, like a checkbox, that suggests a toggle despite the fact that they are one shot actions. Why adding that decoration?

[babluboy]

@memeplex Thanks for taking time to raise this issue. Seems odd, as I cannot see any checkbox on the context menu (screen shot below). What distro are you using and will be great if you can provide a screenshot as well.

[memeplex]

I'm using this package from arch linux:

community/bookworm 1.0.0-2 [installed]
    A simple user centric eBook reader which displays multiple eBooks
formats uniformly

This is the PKGBUILD:

 0 # Contributor: ValHue <vhuelamo at gmail dot
com>
  1
  2 pkgname=bookworm
  3 pkgver=1.0.0
  4 pkgrel=2
  5 pkgdesc='A simple user centric eBook reader which displays multiple
eBooks formats uniformly'
  6 url='https://babluboy.github.io/bookworm'
  7 arch=(x86_64)
  8 license=(GPL3)
  9 depends=(granite poppler-glib python unarchiver unzip webkit2gtk)
 10 makedepends=(cmake vala ninja)
 11 source=(bookworm-$pkgver.tar.gz::
https://github.com/babluboy/bookworm/archive/$pkgver.tar.gz)
 12
sha256sums=('8dacaab3531c189836f7ef65dc69a84de9d03e467e68275bb6c5a31b17f9fbc5')
 13
 14 build() {
 15   cd bookworm-$pkgver
 16   cmake . -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr -G
Ninja
 17   ninja
 18 }
 19
 20 package() {
 21   cd bookworm-$pkgver
 22
 23   DESTDIR="$pkgdir" ninja install
 24   install -m644 -D LICENSE "$pkgdir"/usr/share/licenses/bookworm/LICENSE
 25 }

The desktop is gnome 3.28.

I'm unable to take a screenshot, it seems to immediately close the context menu so I never get it into the picture. It looks like yours but with little squares at the left of each entry.

[fabiocolacio]

I have this same issue on Fedora 28. I managed to take a screenshot:

I am not sure how to reference source code in Github, but I believe this snippet in window.vala is the cause @babluboy :

aWebView.context_menu.connect ((context_menu, event, hit_test_result) => {
                context_menu.remove_all();
                SimpleAction pageActionFullScreenEntry = new SimpleAction.stateful ("FULL_SCREEN_READING_VIEW", 
                                                       null, 
                                                       new Variant.boolean (false)
                                                       );
                SimpleAction pageActionFullScreenExit = new SimpleAction.stateful ("FULL_SCREEN_READING_VIEW", 
                                                       null, 
                                                       new Variant.boolean (false)
                                                       );
                SimpleAction pageActionWordMeaning = new SimpleAction.stateful ("WORD_MEANING", 
                                                       null, 
                                                       new Variant.boolean (false)
                                                       );
                SimpleAction pageActionAnnotateSelection = new SimpleAction.stateful ("ANNOTATE_SELECTION", 
                                                       null, 
                                                       new Variant.boolean (false)
                                                       );
// etc ...

You are using stateful actions for features which need no state. Rather than calling

new SimpleAction.stateful (
    "ANNOTATE_SELECTION", 
    null, 
    new Variant.boolean (false));

you can call:

new SimpleAction(
    "ANNOTATE_SELECTION",
    null);

I actually recompiled with these changes and it solves the issue for me, but if you are using those state values for something that I am unaware of, the fix may cause other issues.

If you would like me to submit a PR, ping be and I will be glad to do so.

[babluboy]

@fabiocolacio Many thanks for your investigation, suggested fix and providing a screen shot of the issue. I was not sure why I had used stateful on SimpleAction, but there is no requirement to keep state on the webview. Not sure why elementary did not show those toggle box on the context menu which is why I was not aware of the issue. Anyways, I have pushed a fix with your suggested changes and it makes no difference on elementary os, hopefully it fixes the issue reported by @memeplex on Arch.

Since fabiocolacio has tested the fix on fedora, I'm marking this as complete, please feel free to open it if the issue re-occurs after this fix.