Hi Nektarios, it seems that this CL failed on some of the ChromeOS and Linux builders.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
After our offline discussion today I simplified the implementation as you suggested.
Thanks!
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Thanks for this CL! Mostly looks good, but I wanted to check with you about the structure for a page without OCR results. Please find my comment below.
ui::AXNodeData root;
What do you think about making it consistent with the existing structure of a tree built with OCR results, which has the page node as its root? Then, we can add the unlabeled_image node to this page node, which would look like: ```
Or we can follow how we create a page node with an unlabeled image node in `PdfAccessibilityTree` for Chrome PDF Viewer, which has the following structure: ```
Let me know if it's better to keep the current implementation, which seems to mimic how we create a tree for a HTML page.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
ui::AXNodeData root;
What do you think about making it consistent with the existing structure of a tree built with OCR results, which has the page node as its root? Then, we can add the unlabeled_image node to this page node, which would look like: ```
- Page node
- - Image node
- ```
Or we can follow how we create a page node with an unlabeled image node in `PdfAccessibilityTree` for Chrome PDF Viewer, which has the following structure: ```
- Page node
- - Paragraph node
- -- Image node
- ```
Let me know if it's better to keep the current implementation, which seems to mimic how we create a tree for a HTML page.
You are perfectly right. I totally forgot that we are creating a page, not a new document. The page will be provided by GenerateDocumentTree so we don't need to include it. Only the paragraph and the unlabeled image needs to be created.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Code-Review | +1 |
Two comments that I think can be quickly fixed; so giving LGTM to unblock.
#include "chrome/grit/generated_resources.h"
nit: I think this header is not needed.
unlabeled_image.relative_bounds.bounds = gfx::RectF(1, 1);
Ideally, it should be the same as the image size. I think we can get that information from `page_metadata_.at(dirty_page_id).rect`? I believe that we just need to use the width and height info from it; that is, no need to set its origin (x,y).
"offset_container_id=1 (0, 0)-(1, 1) restriction=readonly\n",
This would need to be updated if we set the unlabeled image node to be the same as the image size.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Hi Nektarios, it seems that this CL failed on some of the ChromeOS and Linux builders.
Looks like this CL failed on the linux-chromeos-rel builder. Please fix the issue.