Bug 120407 - Resolve unused parameter warning in WebKitAccessibleInterfaceText.cpp
Summary: Resolve unused parameter warning in WebKitAccessibleInterfaceText.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-28 06:19 PDT by Tamas Czene
Modified: 2013-08-30 08:23 PDT (History)
8 users (show)

See Also:

Attachments
proposed patch (1.46 KB, patch)
2013-08-28 06:21 PDT, Tamas Czene 		darin: review-
darin: commit-queue-
Details | Formatted Diff | Diff
proposed patch (1.44 KB, patch)
2013-08-30 02:12 PDT, Tamas Czene 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tamas Czene 2013-08-28 06:19:56 PDT 
Resolve unused parameter warning in WebKitAccessibleInterfaceText.cpp
Comment 1 Radar WebKit Bug Importer 2013-08-28 06:20:37 PDT 
<rdar://problem/14855742>
Comment 2 Tamas Czene 2013-08-28 06:21:56 PDT 
Created attachment 209872 [details]
proposed patch
Comment 3 Darin Adler 2013-08-28 09:41:15 PDT 
Comment on attachment 209872 [details]
proposed patch

Since this function is local to this file, it doesn't make sense to pass and ignore an argument. We should just remove this argument from this function and all the call sites.

I think it’s better style to just remove the argument name entirely rather than commenting it out, but it’s debatable I suppose. I also think the spacing inside the /* */ draws too much attention to the unused argument.
Comment 4 Tamas Czene 2013-08-30 02:12:11 PDT 
Created attachment 210081 [details]
proposed patch

I wouldn't really like to delete it, because the name can refer to its purpose.
Comment 5 Darin Adler 2013-08-30 08:00:53 PDT 
Comment on attachment 210081 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210081&action=review

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:715
> +static VisibleSelection wordAtPositionForAtkBoundary(const AccessibilityObject* /*coreObject*/, const VisiblePosition& position, AtkTextBoundary boundaryType)

You say “the name refers to its purpose”, but “core object” means nothing to me. Core of what? What object? It’s not adding anything.
Comment 6 WebKit Commit Bot 2013-08-30 08:23:20 PDT 
Comment on attachment 210081 [details]
proposed patch

Clearing flags on attachment: 210081

Committed r154882: <http://trac.webkit.org/changeset/154882>
Comment 7 WebKit Commit Bot 2013-08-30 08:23:22 PDT 
All reviewed patches have been landed.  Closing bug.