Last Comment Bug 580520 - Update Comment on mimeTypeIsTextBased to Include Request to Update findbar.xml Upon Any Changes
: Update Comment on mimeTypeIsTextBased to Include Request to Update findbar.xm...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Find In Page (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a3
Assigned To: Ben Frisch
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-20 22:03 PDT by Ben Frisch
Modified: 2010-07-25 19:24 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add the Comment v.1 (1007 bytes, patch)
2010-07-20 22:07 PDT, Ben Frisch
no flags Details | Diff | Splinter Review
Add the Comment v.2 (963 bytes, patch)
2010-07-23 05:01 PDT, Ben Frisch
neil: review+
Details | Diff | Splinter Review

Description Ben Frisch 2010-07-20 22:03:40 PDT
Follow-up bug from bug 97023 comment 51 where Philip Chee wrote:
> In Firefox there is this comment:
> 
> /**
>  * Returns true if |aMimeType| is text-based, false otherwise.
>  *
>  * @param aMimeType
>  *        The MIME type to check.
>  *
>  * If adding types to this function, please also check the similar
>  * function in findbar.xml
>  */
> function mimeTypeIsTextBased(aMimeType)
> 
> Perhaps we should add a similar comment to our implementation of
> mimeTypeIsTextBased()
Comment 1 Ben Frisch 2010-07-20 22:07:56 PDT
Created attachment 458911 [details] [diff] [review]
Add the Comment v.1

This patch should add an appropriate comment.
Comment 2 neil@parkwaycc.co.uk 2010-07-21 04:49:41 PDT
Comment on attachment 458911 [details] [diff] [review]
Add the Comment v.1

>+ * If adding types to this function, please also check the similar
>+ * functions in suite/commom/bindings/findbar.xml and
suite/commom/bindings/findbar.xml just inherits the toolkit version, it doesn't have its own function.

>+ * mozilla/toolkit/widgets/findbar.xml.
Did you mean mozilla/toolkit/content/widgets/findbar.xml?
Comment 3 Ben Frisch 2010-07-23 05:01:01 PDT
Created attachment 459776 [details] [diff] [review]
Add the Comment v.2

This fixed the wrong path to findbar.xml in the toolkit.  It also gets rid of the reference to the findbar.xml in suite, as now that I think about it, we probably shouldn't have a need to update the function in just the suite findbar.xml in the future.

Thanks.
Comment 4 neil@parkwaycc.co.uk 2010-07-23 12:09:18 PDT
Pushed changeset 12adef298038 to comm-central.

Note You need to log in before you can comment on or make changes to this bug.