Skip to content

Fixes list item numbers for ordered lists in Internet Explorer.#8474

Closed
dineshkaushal wants to merge 1 commit into
nvaccess:masterfrom
nvda-india:in-i8438-part1
Closed

Fixes list item numbers for ordered lists in Internet Explorer.#8474
dineshkaushal wants to merge 1 commit into
nvaccess:masterfrom
nvda-india:in-i8438-part1

Conversation

@dineshkaushal

@dineshkaushal dineshkaushal commented Jul 2, 2018

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #8438

Summary of the issue:

NVDA is not reading list item numbers for ordered list if the number is specified in start attribute of a ol tag.

NVDA also does not handle type attribute that specifies what type of numbers should be shown. For example, if type attribute specifies "A" then NVDA should read alphabets.

Description of how this pull request fixes the issue:

This pull request fixes the handling of start attribute for MSHTML.

Testing performed:

Tested with the file attached.

Known issues with pull request:

We still need to work on type attribute. A big challenge is generating numbers in A,B,C format and also generating those numbers in roman format as type attribute could specify roman numerals as well.

Another problem is that the fix is not working for MSHTML control embedded in applications such as vital source.

Change log entry:

Section: Bug fixes

In Internet Explorer, Fixes reading of numbers for ordered lists if the number does not start with 1.
OLHTML.zip

@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.

Can you report on the state of this for other browsers? If the behaviour does not match, do you plan to address the other browsers?

//Ordered lists should number their list items
LIIndex=1;
// set the list index if list has start attribute
if ((tempIter = attribsMap.find(L"HTMLAttrib::start")) != attribsMap.end()) {

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.

While I think this is done a fair bit in this file. Please don't assign a value and make a comparison all in one. It's hard to read and error prone. Please split this into two lines, you can also then check for empty:

tempIter = attribsMap.find(L"HTMLAttrib::start")
if (tempIter != attribsMap.end() && !tempIter->second.empty()) {

// set the list index if list has start attribute
if ((tempIter = attribsMap.find(L"HTMLAttrib::start")) != attribsMap.end()) {
if (!tempIter->second.empty()) {
LIIndex = stoi(tempIter->second);

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.

This could throw either std::invalid_argument or std::out_of_range check https://en.cppreference.com/w/cpp/string/basic_string/stol. Please handle this.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@dineshkaushal: could you please provide updates and address @feerrenrut's questions?

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@dineshkaushal: Could you please address the concerns and questions as formulated above?

@feerrenrut feerrenrut added Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. app/internet-explorer bug labels Apr 8, 2020
@feerrenrut feerrenrut self-assigned this Jul 3, 2020
feerrenrut added a commit that referenced this pull request Jul 7, 2020
@feerrenrut

Copy link
Copy Markdown
Contributor

I'm not able to push to this branch, so I have created a new PR. Closing this as it is superseded by #11346

@feerrenrut feerrenrut closed this Jul 7, 2020
feerrenrut added a commit that referenced this pull request Jul 13, 2020
* adds support for 'start' attribute for 'OL' tag which partially fixes order numbering for numbered lists.
  - Ordered lists that start with a letter or roman numeral will still be announced as digits.
* Review actions for #8474
* No longer treat unordered lists like ordered lists, they don't need an index to count their position.
* Removes unused "type" attribute fetching for "OL" element

Co-authored-by: Dinesh Kaushal <dineshkaushal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. app/internet-explorer bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ordered list numbering always starts from number 1 without regard to type or start attribute in internet explorer

3 participants