Summary:
- Unify all the reference/embed Remarkup rules for Differential, Maniphest, Paste and Ponder.
- Add rules for Pholio.
- Does not yet unify Diffusion or Files (both are a bit more involved).
- Prepare for hovercards.
Test Plan: {F33894}
Reviewers: chad, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5120
Summary:
D5120 and followups refactor and generalize object references in Remarkup -- notably, they move remarkup rules from a central location to the implementing applications.
Preserve blame by doing moves/renames only first. This change moves application remarkup rules into those applications, and renames the ones D5120 modifies.
Test Plan: Typed some preview text into a textarea, got a valid Remarkup render.
Reviewers: vrana, chad
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5123
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.
Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4927
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary: `renderChildren()` now returns array which isn't ideal but I prefer it to having two methods.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4910
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4905
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
Summary:
Convert a final few `render_tag()` calls to `tag()` calls. This leaves us with 36 calls:
- 9 are in Conpherence, and will be converted after merging. Using Views makes the most sense here, to get access to renderHTMLView() and such.
- 2 are the definition and its library map entry.
- 3 are in the documentation.
- I believe the remaining 22 are too difficult to convert pre-merge. About half are in code which is slated for destruction; the other half are in the base implementations of very common classes (like PhabricatorStandardPageView) and can only be converted by converting the entire codebase.
My plan at this point is:
- Test the branch thoroughly.
- Merge to master.
- Over time, resolve the remaining issues: lint means things shouldn't get any worse.
Test Plan: See inlines.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4802
Summary: This works after pht() + html got sorted out.
Test Plan: Looked at some object attribute lists.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4645
Summary:
- Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
- Manually converts all or almost all of the trivial callsites.
Test Plan:
- Site does not seem any more broken than before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4639
Summary: Fixes T2323. Prior to search refactoring (I think), this stuff was loaded implicitly. Now load it explicitly.
Test Plan: Added a comment to a Ponder answer without an exception.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2323
Differential Revision: https://secure.phabricator.com/D4536
Summary: See rP883829e6676fc3412b83b6ab16f7bf5b56b174b8
Test Plan: Verified no XSS with a title like `<b>!</b>`.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4534
Summary: Not all applications' glyphs survived the migration. Restore them.
Test Plan: Looked at Differential, Phriction, Ponder. Saw title glyphs in page titles.
Reviewers: lesha, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4489
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.
Test Plan: Go to Applications, click on every application.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4420
Summary:
The search indexing API has several problems right now:
- Always runs in-process.
- It would be nice to push this into the task queue for performance. However, the API currently passses an object all the way through (and some indexers depend on preloaded object attributes), so it can't be dumped into the task queue at any stage since we can't serialize it.
- Being able to use the task queue will also make rebuilding indexes faster.
- Instead, make the API phid-oriented.
- No uniform indexing API.
- Each "Editor" currently calls SomeCustomIndexer::indexThing(). This won't work with AbstractTransactions. The API is also just weird.
- Instead, provide a uniform API.
- No uniform CLI.
- We have `scripts/search/reindex_everything.php`, but it doesn't actually index everything. Each new document type needs to be separately added to it, leading to stuff like D3839. Third-party applications can't provide indexers.
- Instead, let indexers expose documents for indexing.
- Not application-oriented.
- All the indexers live in search/ right now, which isn't the right organization in an application-orietned view of the world.
- Instead, move indexers to applications and load them with SymbolLoader.
Test Plan:
- `bin/search index`
- Indexed one revision, one task.
- Indexed `--type TASK`, `--type DREV`, etc., for all types.
- Indexed `--all`.
- Added the word "saboteur" to a revision, task, wiki page, and question and then searched for it.
- Creating users is a pain; searched for a user after indexing.
- Creating commits is a pain; searched for a commit after indexing.
- Mocks aren't currently loadable in the result view, so their indexing is moot.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: 20after4, aran
Maniphest Tasks: T1991, T2104
Differential Revision: https://secure.phabricator.com/D4261
Summary: This is used in every other view.
Test Plan: Browsed around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4248
Summary:
Add a `setViewer()` so that PHID loading doesn't throw.
Also remove a period from a button because none of the others have one.
Test Plan: Saved a comment successfully.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4202
Summary: See discussion in T2014. Aligns this element more closely with @chad's `frame_v3.psd` mock, and implements the icon/label element. Removes "details".
Test Plan: {F27062} {F27063} {F27064} {F27065}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2014
Differential Revision: https://secure.phabricator.com/D4179
Summary: wanted to play with some policy stuff as its been a bit. Turns out you can't edit questions so this is very silly "so long as you are a user you can view it" policy. also sorry if you have a diff or twelve out for this in your sandbox(es).
Test Plan: loaded up ponder and clicked about
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2113
Differential Revision: https://secure.phabricator.com/D4163
Summary:
Issues here:
- Need an application-sized "eye", or a "home" icon for "Phabricator Home".
- Some of the "apps_lb_2x" sliced images are the "_dark_" versions, not the light versions.
- If you slice an application-sized "logout" (power off) icon and application-sized "help" (questionmark in circle) icon I can replace the current menu icons and nearly get rid of "autosprite".
- To replace the icons on /applications/, the non-retina size is "4x", so we'd need "8x" for retina. Alternatively I can reduce the icon sizes by 50%.
- The "Help", "Settings" and "Logout" items currently have a "glowing" hover state, which needs a variant (or we can drop it).
- The /applications/ icons have a white hover state (or we can drop it).
- The 1x application (14x14) icons aren't used anywhere right now, should they be? Maybe in the feed in the future, etc?
- The "apps-2x" and "apps-large" sheets are the same image, but getting them to actually use the same file is a bit tricky, so I just left them separate for now.
Test Plan:
{F26698}
{F26699}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4108
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.
Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2114
Differential Revision: https://secure.phabricator.com/D4037
Summary: was poking at T654 and noticed subscribers weren't exposed in search UI so I did so. Also make ponder a little less silly on the double handles load. Finally, stopped showing the "Examine Index" link to non admins since they can't click it. Note this introduces a UI oddity in that you Users and Phriction Documents don't currently have the subscribe functionality.
Test Plan: searched for subscribers in all applications - it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3907
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.
Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, vrana
Maniphest Tasks: T1676
Differential Revision: https://secure.phabricator.com/D3645
Summary:
We need to go slightly farther to stub reply handler functionality for Ponder in at least some configurations, where we rely on the presence of a unique random key to generate per-object or per-object+user reply addresses.
This should probably be formalized in an interface since it's currently pretty ad-hoc.
Test Plan:
- Made comments in Ponder under a per-user email configuration.
- Ran migration, verified mail keys were generated.
- Ran migration again (with --apply), verified existing questions were skipped.
- Created a new question, verified mail key generation.
Reviewers: pieter
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1873
Differential Revision: https://secure.phabricator.com/D3665
Summary: Make these always work. Notably, this makes them work in Maniphest. Previously this was at odds with stuff fixed in D3651.
Test Plan: Dragged and dropped files into Remarkup in Maniphest.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3652
Summary:
* Moved the remaining `->save()` calls into editors
* for the `PonderQuestion`, separate `attachRelated` (needed for
indexing and viewing) from `attachVotes` (needed for viewing
but not indexing)
* Update the indexer to include comment and answer content
in the search index.
Test Plan:
Stuff works as before. Also: add a comment or answer with some
easily-identifiable text and search for it; observe that
it gets indexed appropriately.
Reviewers: epriestley, nh, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1869
Differential Revision: https://secure.phabricator.com/D3654
Summary:
D03646 works, I don't want it to work.
Theoretically, it can cause us some troubles if we use this string in JS number context where 030 is 24.
Test Plan: D03646, D3646
Reviewers: epriestley, edward
Reviewed By: edward
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3646
Summary: This is sort of silly as-is, but automatically exposes flagging and will give subscribe/unsubscribe and "Subscribers" a place to plug into shortly. For context, see D3637 and T1808.
Test Plan: {F20550}
Reviewers: pieter, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1808
Differential Revision: https://secure.phabricator.com/D3641
Summary: also did a wee bit o' formatting stuff while I was in there.
Test Plan: it looks... well, it looks like its using the new UI component and all the information is there!
Reviewers: epriestley, pieter
CC: aran, Korvin
Maniphest Tasks: T1845
Differential Revision: https://secure.phabricator.com/D3611
Summary: So they're maybe a little easier to deal with? I'm going to take this formally to "plz @chad plz help" land.
Test Plan: {F20329}
Reviewers: btrahan, vrana, chad
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3609
Summary:
Use sigils to simplify the vote implementation and move most rendering to the server.
Use unicode glyphs in place of graphics.
Test Plan: {F19539}
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3518
Summary: No use sites left after D3513.
Test Plan: `grep`
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3515
Summary: Restore the pager, using `executeWithOffsetPager()` to handle slicing, etc. Simplify and generalize `PonderQuestionQuery`.
Test Plan:
Set page size to 1, used pager to page.
{F19531}
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3513
Summary: The aoff/qoff thing is pretty awkward and putting these both on the same page is probably only at all useful when looking at someone else's questions/answers -- we should just pursue main profile integration for that.
Test Plan: {F19529}
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3512
Summary: Use flexible form and application side nav.
Test Plan: {F19525}
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3511
Summary:
Use the new `PhabricatorObjectItemListView` in Ponder so it works with the new UI. It will also get some features like flags "for free" in the future.
This removes the pager; I'll restore it in the next diff.
Test Plan: Looked at feed.
Reviewers: pieter, starruler
Reviewed By: pieter
CC: aran, chad
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3507
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.
Test Plan: played around with each tool and verified the Remarkup reference was present
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1756
Differential Revision: https://secure.phabricator.com/D3468
Summary:
Any answer without non whitespace characters results in an
error.
Test Plan:
- Submitted an empty answer, was told off
- Submitted an answer full of whitespace, didn't like that either
- Submitted a real answer, accepted
Reviewers: pieter, epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin, davidreuss
Maniphest Tasks: T1644
Differential Revision: https://secure.phabricator.com/D3492
Summary: Question titles were not escaped; now they are.
Test Plan: Observe the escaping.
Reviewers: epriestley
Reviewed By: epriestley
CC: nh, aran, Korvin
Differential Revision: https://secure.phabricator.com/D3490
Summary: Does what it says on the tin
Test Plan: Viewed ponder question, expanded link, added comment
Reviewers: pieter, epriestley
Reviewed By: pieter
CC: vrana, aran, Korvin
Maniphest Tasks: T1775
Differential Revision: https://secure.phabricator.com/D3485
Summary: This is pretty spartan, but it does the job.
Test Plan:
Patch, update storage, add some comment
to your favorite question or answer.
Reviewers: nh, vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin, starruler, syrneus, me.here, victorzarate7
Maniphest Tasks: T1645
Differential Revision: https://secure.phabricator.com/D3471
Summary: AphrontSideNavView is oldschool and I want to kill it.
Test Plan: Viewed Ponder, clicked both nav options.
Reviewers: pieter, vrana, btrahan
Reviewed By: pieter
CC: aran
Differential Revision: https://secure.phabricator.com/D3430
Summary:
I want to implement a `{P123}` rule to embed pastes, but we need viewers everywhere before it will work with privacy.
This is not exhaustive; many Remarkup callsites haven't been converted to `PhabricatorMarkupInterface` yet.
Test Plan: Looked at Maniphest, Differential, Diffusion, Phriction; added markup, made edits and hit previews.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3428
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.
Test Plan: Displayed revision with sporadic author.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3432
Summary:
Ponder is similar in spirit to the Wiki tool, but uses a Q&A
format and up/downvotes to signal user sentiment. Popular
questions are moved to the top of the feed on a 5-minute
cycle based on age (younger is better) and vote count (higher
is better).
Pre-apologies for noob diff.
Test Plan:
- `./bin/phd list` Should include `PonderHeatDaemon`; phd launch it
if necessary.
- Navigate to /ponder/ ; observe sanity when adding questions,
voting on them, and adding answers.
- Confirm that questions and answers are linkable using Q5 / Q5#A5 formatted object links.
- Confirm that searching for Ponder Questions works using built-in
search.
Feedback on code / schema / whatever organization very welcome.
Reviewers: nh, vrana, epriestley
Reviewed By: epriestley
CC: gmarcotte, aran, Korvin, starruler
Differential Revision: https://secure.phabricator.com/D3136
