Closed
Bug 1087190
Opened 10 years ago
Closed 10 years ago
[Text selection] Add debug logs to SelectionCarets
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: TYLin, Assigned: TYLin)
Details
Attachments
(1 file, 5 obsolete files)
13.65 KB,
patch
|
TYLin
:
review+
|
Details | Diff | Splinter Review |
It will be easier to debug SelectionCarets with some logs.
Assignee | ||
Comment 1•10 years ago
|
||
Morris, If there're some critical points in SelectionCarets that should have a log print, feel free to let me know.
Attachment #8509307 -
Flags: feedback?(mtseng)
Comment 2•10 years ago
|
||
Comment on attachment 8509307 [details] [diff] [review] Add debug log to SelectionCarets. Review of attachment 8509307 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/SelectionCarets.cpp @@ +1032,5 @@ > nsRefPtr<SelectionCarets> self = static_cast<SelectionCarets*>(aSelectionCarets); > NS_PRECONDITION(aTimer == self->mLongTapDetectorTimer, > "Unexpected timer"); > > + SELECTIONCARETS_LOG_STATIC("SelectWord"); This long tap event would be fired if apzc is not available. for apzc case, we'll receive event from apzc and handle it at SelectionCarets::HandleEvent(). So, please add a log for apzc long tap as well and both log message should able to distinguish where is long tap coming from.
Attachment #8509307 -
Flags: feedback?(mtseng) → feedback+
Comment 3•10 years ago
|
||
Comment on attachment 8509307 [details] [diff] [review] Add debug log to SelectionCarets. Review of attachment 8509307 [details] [diff] [review]: ----------------------------------------------------------------- I suggest to add the log for SelectWord, but the GetFrameName is required to define DEBUG_FRAME_DUMP. 111 SetSelectionDragState(true); 112 nsFrame* frame = static_cast<nsFrame*>(ptFrame); 113 nsresult rs = frame->SelectByTypeAtPoint(mPresShell->GetPresContext(), ptInFrame, 114 eSelectWord, eSelectWord, 0); 115 + frame->GetFrameName(fName); 116 + printf_stderr("peter SelectionCarets::SelectWord(%d) ptFrame %p ptFrameyType %s ptInFrame %f %f flags 0x%08x\n", 117 + __LINE__, frame, NS_ConvertUTF16toUTF8(fName).get(), (float)ptInFrame.x, (float)ptInFrame.y, flags);
Assignee | ||
Comment 4•10 years ago
|
||
Addressed comment 2 and comment 3.
Attachment #8510124 -
Flags: review?(roc)
Comment on attachment 8510124 [details] [diff] [review] Add debug log to SelectionCarets. f=mtseng, r=roc Review of attachment 8510124 [details] [diff] [review]: ----------------------------------------------------------------- Please use NSPR logging instead of rolling your own framework.
Attachment #8510124 -
Flags: review?(roc) → review-
Assignee | ||
Comment 6•10 years ago
|
||
Use NSPR logging.
Attachment #8509307 -
Attachment is obsolete: true
Attachment #8510124 -
Attachment is obsolete: true
Attachment #8510963 -
Flags: review?(roc)
Assignee | ||
Comment 7•10 years ago
|
||
To export NSPR_LOG_MODULES=SelectionCarets:5 and enable NSPR log on B2G, see this document. https://developer.mozilla.org/en-US/Firefox_OS/Developing_Firefox_OS/Customizing_the_b2g.sh_script
Comment on attachment 8510963 [details] [diff] [review] Add debug log to SelectionCarets. f=mtseng, r=roc (v2) Review of attachment 8510963 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!!!
Attachment #8510963 -
Flags: review?(roc) → review+
Comment 9•10 years ago
|
||
Comment on attachment 8510963 [details] [diff] [review] Add debug log to SelectionCarets. f=mtseng, r=roc (v2) Review of attachment 8510963 [details] [diff] [review]: ----------------------------------------------------------------- With this patch, I could dump the selectioncaret log in B2G. ::: layout/base/SelectionCarets.cpp @@ +941,5 @@ > nsISelection* aSel, > int16_t aReason) > { > + SELECTIONCARETS_LOG("Reason=%d", aReason); > + Please also dump the collapse state here.
Assignee | ||
Comment 10•10 years ago
|
||
Peter, I'll fixup this patch to my main patch if it's look good to you.
Attachment #8512692 -
Flags: feedback?(pchang)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Comment 11•10 years ago
|
||
Comment on attachment 8512692 [details] [diff] [review] Use __FUNCTION__ and add log about collapsed. f=pchang Review of attachment 8512692 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8512692 -
Flags: feedback?(pchang) → feedback+
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8512692 [details] [diff] [review] Use __FUNCTION__ and add log about collapsed. f=pchang Review of attachment 8512692 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/SelectionCarets.cpp @@ +950,4 @@ > if (isCollapsed) { > SetVisibility(false); > return NS_OK; > } This was removed in bug 1074736. I'll print the collapsed info in UpdateSelectionCarets()
Assignee | ||
Comment 13•10 years ago
|
||
Rebased.
Attachment #8510963 -
Attachment is obsolete: true
Attachment #8512692 -
Attachment is obsolete: true
Attachment #8513224 -
Flags: review+
Assignee | ||
Comment 14•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=09cd7da02ca9
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 15•10 years ago
|
||
s/kLogModuleName/kSelectionCaretsLogModuleName so that it wont't clash with names in other cpp in unified build in the future.
Attachment #8513224 -
Attachment is obsolete: true
Attachment #8513302 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/468517afd116
Keywords: checkin-needed
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/468517afd116
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•