Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2855043003:
Added some methods to abstract inline text boxes. (Closed)
|
Created:
3 years, 7 months ago by nektarios Modified:
3 years, 7 months ago CC:
aboxhall, aboxhall+watch_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, dmazzoni+watch_chromium.org, dmazzoni, dougt+watch_chromium.org, dtseng+watch_chromium.org, eae+blinkwatch, haraken, jchaffraix+rendering, je_julie, leviw+renderwatch, nektar+watch_chromium.org, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, yuzo+watch_chromium.org, zoltan1 Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdded some methods to abstract inline text boxes.
GetNode, IsAXInlineTextBox, Detach.
These will most likely be needed for adding SetSelection support to this class.
R=dmazzoni@chromium.org, cbiesinger@chromium.org
Review-Url: https://codereview.chromium.org/2855043003
Cr-Commit-Position: refs/heads/master@{#471148}
Committed: https://chromium.googlesource.com/chromium/src/+/bdf09ee24f27a98dd258d6b644a4341f4abd4414
Patch Set 1 #
Messages
Total messages: 33 (15 generated)
lgtm These seem fine, but it'd help to understand the purpose or greater context for why you're making these changes. Could you add another sentence to the change description? I'm assuming that these are necessary prerequisites to making it possible to call SetSelection on an AXInlineTextBox.
ikilpatrick@chromium.org changed reviewers: + ikilpatrick@chromium.org
+1 for greater detail in change description.
Description was changed from ========== Added some methods to abstract inline text boxes. GetNode, IsAXInlineTextBox, Detach. R=dmazzoni@chromium.org, cbiesinger@chromium.org ========== to ========== Added some methods to abstract inline text boxes. GetNode, IsAXInlineTextBox, Detach. These will most likely be needed for adding SetSelection support to this class. R=dmazzoni@chromium.org, cbiesinger@chromium.org ==========
SetSelection support was the reason I sent out this patch. However, my plans for SetSelection have not been finalized, and I didn't want to write something that might be false. Anyway, I added a sentense.
Should we check this patch in?
The CQ bit was checked by nektar@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
lgtm for core/ OWNER
The CQ bit was checked by cbiesinger@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by nektar@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by nektar@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by nektar@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by nektar@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1494541090566670, "parent_rev":
"df9197b54026160887760b92a9500111a2b121b3", "commit_rev":
"bdf09ee24f27a98dd258d6b644a4341f4abd4414"}
Message was sent while issue was closed.
Description was changed from ========== Added some methods to abstract inline text boxes. GetNode, IsAXInlineTextBox, Detach. These will most likely be needed for adding SetSelection support to this class. R=dmazzoni@chromium.org, cbiesinger@chromium.org ========== to ========== Added some methods to abstract inline text boxes. GetNode, IsAXInlineTextBox, Detach. These will most likely be needed for adding SetSelection support to this class. R=dmazzoni@chromium.org, cbiesinger@chromium.org Review-Url: https://codereview.chromium.org/2855043003 Cr-Commit-Position: refs/heads/master@{#471148} Committed: https://chromium.googlesource.com/chromium/src/+/bdf09ee24f27a98dd258d6b644a4... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/bdf09ee24f27a98dd258d6b644a4... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
