Skip to content

Abbreviate landmark names in braille#6813

Merged
feerrenrut merged 6 commits into
nvaccess:masterfrom
dkager:i3975
May 30, 2017
Merged

Abbreviate landmark names in braille#6813
feerrenrut merged 6 commits into
nvaccess:masterfrom
dkager:i3975

Conversation

@dkager

@dkager dkager commented Jan 26, 2017

Copy link
Copy Markdown
Contributor

Re #3975

Summary of proposed changes:
The order of the word landmark and the type of landmark is swapped. Landmark becomes "lmk". The names of each of the landmarks are altered as follows:

"banner": "bnnr"
"complementary": "comp"
"contentinfo": "cinf"
"main": "main"
"navigation": "nav"
"search": "srch"
"form": "frm"
"region": "rgn"

Example: "navigation landmark" becomes "lmk nav".

…yed in braille. Third part of nvaccess#3975.

Rationale: the word landmark is the more significant part. That is, the word landmark indicates that we are dealing with a landmark while the landmark name describes the specific type.
@feerrenrut

Copy link
Copy Markdown
Contributor

Thanks for the contribution!
Also please could you outline the testing that you have done on this. Ideally I would love to see a test case document.

@dkager

dkager commented Apr 27, 2017

Copy link
Copy Markdown
Contributor Author

Testing was mostly just running this code in the wild. The patch is simple enough to reason about its correctness. There are no good standards for these abbreviations. So I tested this by simply using it a few days. This resulted in changing the abbreviation for banner from "bnr" to "bnnr". I'm hoping that its incubation will reveal any remaining issues, though I don't think there will be any.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why did you change banner to "bnnr"? Have you tried output of all of the other landmarks?

Please can you update the user guide with these changes too.

@dkager

dkager commented Apr 28, 2017

Copy link
Copy Markdown
Contributor Author

Out of curiosity, why did you change banner to "bnnr"?

Because it felt a bit more intuitive, based on emperical testing by me and @LeonarddeR. We also thought it would be good if most landmark names had a consistent 4-character length. There are some exceptions. For example, we couldn't agree on a good 4-character abbreviation for region. Looking back at it now, I also feel we should use form instead of frm, because the latter could be mistaken to mean frame.

Have you tried output of all of the other landmarks?

Yes. This includes the somewhat special case of region, which isn't strictly a landmark.

dkager added 2 commits April 28, 2017 11:01
  - frm --> form, to avoid confusion with the word frame.
  - comp --> cmpl, to avoid confusion with the many words starting with comp.
  - nav --> navi, so all landmark names are 4 characters long.
Only region now is a 3-character abbreviation. This should be OK because:
  * There is no good 4-character abbreviation. The current abbreviation (rgn)
    follows the common convention of excluding all vowels from the word.
  * It is strictly speaking not a landmark.
@dkager

dkager commented Apr 28, 2017

Copy link
Copy Markdown
Contributor Author

Updated some landmark abbreviations (see commit message) and added them to the user guide.

I don't have the lists of websites anymore that I used for testing. But one could also write a short HTML document containing all landmarks and verify that the abbreviations come out correctly. I did have a short test document for named landmarks, but can't find it at the moment.

@dkager

dkager commented May 3, 2017

Copy link
Copy Markdown
Contributor Author

@feerrenrut Here is a test HTML document that has all the landmarks in it. I also had a stab at writing a unit test, but that's not so straight-forward because of all the TextInfo stuff.

nvda-braille-landmark-test.html.txt

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates.

feerrenrut added a commit that referenced this pull request May 4, 2017
Merge branch 'dkager-i3975' into next
@feerrenrut feerrenrut merged commit a6487ef into nvaccess:master May 30, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.3 milestone May 30, 2017
feerrenrut added a commit that referenced this pull request May 30, 2017
- PR #7169 : Editable div elements in Chrome are no longer have their label reported as their value while in browse mode. (Issue #7153)
- PR #6396 : An unbound gesture (script_restart) has been added to allow NVDA to be restarted quickly. (PR #6396)
- PR #6777 : A Braille setting has been added to "show messages indefinitely". (Issue #6669)
- PR #7133 : Pressing end while in browse mode of an empty Microsoft Word document no longer causes a runtime error. (Issue #7009)
- PR #6868 : The keyboard layout can now be set from the NVDA Welcome dialog. (Issue #6863)
- PR #6813 : The names of "landmarks" are abbreviated in Braille (Issue #3975)
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.

3 participants