Summary: If we hit an exception very early, the request may not have a user yet. Don't assume it will.
Test Plan: Broke MySQL config on purpose to test some stuff, loaded Phabricator, got a nice error page instead of a super-fatal one.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7286
Summary: Depends on D6769, removes 'dust' and uses a similar color background.
Test Plan: Review colors in sandbox.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6772
Summary:
Ref T3684. The URI itself is reflected in a few places. It is generally not dangerous because we only let you add random stuff to the end of it for one or two controllers (e.g., the file download controller lets you add "/whatever.jpg"), but:
- Remove it entirely in the main request, since it serves no purpose.
- Remove query parameters in Ajax requests. These are available in DarkConsole proper.
Also mask a few things in the "Request" tab; I've never used these fields when debugging or during support, and they leak quasi-sensitive information that could get screenshotted or over-the-shoulder'd.
I didn't mitgate `__metablock__` because I think the threat is so close to 0 that it's not worthwhile.
Test Plan: Used Darkconsole, examined Requests tab.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3684
Differential Revision: https://secure.phabricator.com/D6699
Summary: Rough pass at a PHUIButtonView Class. Keeps phutil_tag intact and adds some image features if you use the class.
Test Plan: UIExamples
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6192
Summary:
This diff covers a bit of ground.
- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView
Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.
Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6130
Summary: 1) Borders were appearing on inputs not as expected. 2) the SWF container was always displayed at the bottom of every page load (long time issue). There are more issues, but this fixes the 2 largest for right now.
Test Plan: Tested Maniphest create page.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5867
Summary:
This creates a common form look and feel across the site. I spent a bit of time working out a number of kinks in our various renderings. Some things:
- Font Styles are correctly applied for form elements now.
- Everything lines up!
- Selects are larger, easier to read, interact.
- Inputs have been squared.
- Consistant CSS applied glow (try it!)
- Improved Mobile Responsiveness
- CSS applied to all form elements, not just Aphront
- Many other minor tweaks.
I tried to hit as many high profile forms as possible in an effort to increase consistency. Stopped for now and will follow up after this lands. I know Evan is not a super fan of the glow, but after working with it for a week, it's way cleaner and responsive than the OS controls. Give it a try.
Test Plan: Tested many applications, forms, mobile and tablet.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5860
Summary:
I feel that remarkup code blocks are still kind of small. This should help with my reading problems. Hesitating to add `.PhabricatorMonospaced` to the remarkup rule, since it's pretty specific to Phabricator.
Pasting in the diff btw, since my `arc`'s going crazy (doubt that's `arc.exe`, since it's not landed yet).
Test Plan:
Set font to something large. viewed code block. Was not large.
Applied change. Viewed code block again. It was //**HUGE**//!
Also verified that the `init.js` was still there. I think that's the one being inserted `$5`.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5727
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary: Adds ability to add 20, 10 and 5 px of margin or padding without adding additional rules.
Test Plan: Reloaded any Phabricator Page. It didn't break.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5477
Summary:
Ref T2700. Allow JS to listen for swipes on devices.
There are a bunch of tricky cases here and I probably didn't get them all totally right, but this interaction broadly looks like this:
- We implement gesture recognition for the mouse in device modes (narrow browser), and for touch events from an actual device.
- The sigil `touchable` indicates that a node wants to react to touch events.
- When the user touches a `touchable` node, we start listening for moves. They might be tapping/clicking (in which case we don't care), but they might also be gesturing.
- Once the user moves their finger/pointer far enough away from the tap origin, we recognize it as a gesture. I hardcoded this at 20px; I wasn't able to find any "official" Apple value, but 20px seems like a common default.
- At this point, we look at where their finger has moved.
- If they moved it mostly up/down, we interpret the gesture as "scroll" and just stop listening. The device does its own thing.
- However, if they moved it mostly left/right, we interpret it as a "swipe". We start killing the moves so the device doesn't scroll.
- Once we've recognized that a gesture is underway, we send a "gesture.swipe.start" event and then "gesture.swipe.move" events for every move.
- When the user ends the gesture, we send "gesture.swipe.end".
- If the user cancels the gesture (currently, only by tapping with a second finger), we send "gesture.swipe.cancel".
- Gesture events have raw position data and some convenience fields.
Test Plan:
Wrote UI example and used it from the Desktop, iPhone simulator, and a real iphone.
- The code always seems to get "scroll" vs "swipe" correct (i.e., consistent with my intentions).
- The threshold feels pretty good to me.
- Tapping with a second finger cancels the action.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2700
Differential Revision: https://secure.phabricator.com/D5308
Summary:
For a single line, I can use the right click.
But for highlighting multiple lines, I have to wait for page reload.
It would be nice to track this in history but that's more involved.
This is actually maybe better behavior.
Test Plan: Clicked on the line link, dragged and dropped on it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5172
Summary: This adds an option dust background for certain application designs, like Macro and Pholio to help make the list views pop more.
Test Plan: Reviewed Macro and Pholio.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5209
Summary:
actually just wanted to `pht()` the //Cancel// button.
includes `pht()`ed exceptions.
refrained from `pht()`-ing `phabricator_relative_date()` since I wasn't too sure about the effects.
Test Plan: visited all form elements that I remembered - looks reasonable
Reviewers: epriestley, chad, btrahan
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5175
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:
- Separate the ideas of "requested" (explicit user request) vs "started" (user request or sampling).
- Move this code out of index.php into the XHProf stuff (general effort to make index.php smaller).
Test Plan:
Verified that profiling still works, and profiling extends to ajax requests.
Set sampling rate to 2, saw 50% samples.
Looked at database, saw sampling data populating properly.
Reviewers: vrana, nh
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4891
Summary: Route all `$_SERVER['HTTP_...']` stuff through AphrontRequest (it would be nice to make this non-static, but the stack is a bit tangled right now...)
Test Plan: Verified CSRF and cascading profiling. `var_dump()`'d User-Agent and Referer and verified they are populated and returned correct values when accessed. Restarted server to trigger setup checks.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4888
Summary: If a page is profiled, add an "X-Phabricator-Profiler" header to all Ajax requests, and profile those too.
Test Plan: Profiled a page, checked Darkconsole, saw profiles for everything.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4885
Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.
Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4780
Summary: Convert most phabricator_render_form callsites. In the case of the "headsup view", it converts it by deleting the element entirely (this is the very old Maniphest/Differential header which we no longer use).
Test Plan: Poked around a bit.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4726
Summary:
This accomplishes three major goals:
# Fixes phutil_render_tag -> phutil_tag callsites in DarkConsole.
# Moves the Ajax request log to a new panel on the left. This panel (and the tabs panel) get scrollbars when they get large, instead of making the page constantly scroll down.
# Loads the panel content over ajax, instead of dumping it into the page body / ajax response body. I've been planning to do this for about 3 years, which is why the plugins are architected the way they are. This should make debugging easier by making response bodies not be 50%+ darkconsole stuff.
Additionally, load the plugins dynamically (the old method predates library maps and PhutilSymbolLoader).
Test Plan:
{F30675}
- Switched between requests and tabs, reloaded page, saw same tab.
- Used "analyze queries", "profile page", triggered errors.
- Verified page does not load anything by default if dark console is closed with Charles.
- Generally banged on it a bit.
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4692
Summary:
I broke this a bit a few revisions ago by making `phabricator_render_csrf_magic()` return array instead of string without actually grepping for callsites.
Instead of building a form in JS, build it in PHP and just change its action in JS. This is simpler and doesn't require us to expose CSRF magic in more places.
This change triggered a rather subtle bug: if we rendered a normal form on the page (and thus installed the "disable forms when submitted" behavior), the download button would incorrectly disable after being clicked. This doesn't currently happen in Files because we don't put a form on the same page as the download button.
Instead, make the "disable" behavior check if the form is downloading stuff, and not disable stuff if it is. Then mark the lightbox and Files form as both download forms.
Test Plan: Used lightbox; used Files. Verified download buttons download the right stuff and behave correctly. Grepped for other download forms, but all the other places where we write "download" don't appear to actually be download links.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4685
Summary:
- Grepped for phutil_render_tag().
- Fixed some easy ones.
Test Plan:
- Browsed around; site didn't seem more broken than it was before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4638
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary: Fixes T2330. Deletes the last traces of the footer.
Test Plan: Looked at several pages, saw no footer. Grepped for footer function and CSS class.
Reviewers: DeedyDas, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2330
Differential Revision: https://secure.phabricator.com/D4554
Summary: Just removed the link and created a new field under preferences. Now the setting is under Display Preferences.
Test Plan: Enablied/Disabled dark console to see if it works.
Reviewers: epriestley
Reviewed By: epriestley
CC: irinav, aran, Korvin
Maniphest Tasks: T2344
Differential Revision: https://secure.phabricator.com/D4549
Conflicts:
src/view/page/PhabricatorStandardPageView.php
Summary: Fixed T2349
Test Plan:
Could not visibly see version at footer any more. Appeared in the top of /config.
Does not appear as a config option in /config.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2349
Differential Revision: https://secure.phabricator.com/D4539
Summary:
Migrate to the new hotness. Also:
- Remove a string test, which is now impossible since the config will repair itself and raise a type error.
- Restore the header even in /config/ -- this check is kind of hacky and it feels a bit more natural now that it's above the menu.
Test Plan: Set my local disk path to something invalid, verified I got a setup error.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2228
Differential Revision: https://secure.phabricator.com/D4497
Summary: Removed the 'Report a Bug' link from the footer
Test Plan: Tried looking through all the phabricator pages but didn't quite get around to it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4528
Summary: We now issue only valid setup warnings, so we can let administrators know when we detect problems.
Test Plan:
Banner:
{F29568}
Created a fake issue; saw banner. No banner inside /config/. Resovled the issue, banner went away.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2221
Differential Revision: https://secure.phabricator.com/D4493
Summary:
- Adds the rest of the group as per T2255.
- Adds a pht() around the `$developer_warning` in `PhabricatorStandardPageView`.
Test Plan:
- Viewed new config options.
- Triggered a fake warning to make sure I didn't break error callouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4333
Summary: We have enough z-index rules that they're fairly hard to visualize with "git grep". Consolidate them. Then fix T2253 (missing z-index on left menu background).
Test Plan: Made a Differential window really narrow, then scrolled it horizontally.
Reviewers: btrahan, chad, ender
Reviewed By: chad
CC: aran
Maniphest Tasks: T2253
Differential Revision: https://secure.phabricator.com/D4302
Summary:
Allows you to edit or delete comments in appplications which support ApplicationTransactions.
UI/UX stuff:
- The dialogs are rough but I want to do a dialog design pass more generally, @chad has some mocks.
- When you add new mentions via edit, they don't currently count as mentions. I'm not sure what I want to do about this.
- When you edit or delete a comment, we do not publish any notifications about it. I think this is reasonable.
- I didn't separate "delete" out versus "edit"; I assume it will be reasonably intuitive that deleting all the text deletes effectively deletes the comment. I also want to discourage deletion somewhat (we still show the transaction, just show that the comment has been deleted).
Test Plan:
Transaction view, note "Edit" and "Edited" links:
{F26914}
Edit view, has some design issues but I want to do a pass on dialogs in general:
{F26915}
History view:
{F26913}
Reviewers: vrana, btrahan, chad
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1082
Differential Revision: https://secure.phabricator.com/D4149
Summary: I think we've sorted out enough of the problems with these to turn them on for everyone. The real-time component remains configuration-dependent.
Test Plan: Turned off "notification.enabled", still saw notifications.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4120
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: Add a ".device" rule which means "phone or tablet". Simplify about 5000 rules which were written ".device-phone X, device-tablet X { ... }".
Test Plan: Browsed the site a bit without incident.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4103
Summary:
This doesn't lay in everything, but:
- Break the buttons gradient apart into components and rebuild it (along with other gradients) into a single gradient sprite (possible after {D4099}).
- Use the sliced gradient for the crumbs background.
- Use the sliced image for the crumb divider.
- Adds the black/white app sheets, but I'm not generating them quite yet.
Test Plan: {F26537} {F26540}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4100
Summary:
Adds a right-hand-side application menu, based roughly on `frame_v3.png`.
This has the same icon as the left menu until we get real design in, but is functionally reasonable.
Test Plan: {F26170} {F26169}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4061
Summary:
As per discussion, this primes the existing mobile menu / menu button for "phabricator" and "application" menus.
Design here is very rough, I'm just trying to get everything laid in functionally first. It's based on `frame_v3.png` but missing a lot of touches.
Test Plan:
{F26143}
{F26144}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4058
Summary: defensive program wins all battles
Test Plan: lightbox still worked
Reviewers: vrana, epriestley
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3975
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:
images attached to maniphest tasks and mentioned in remarkup anywhere now invoke a lightbox control that lets the user page through all the images.
lightbox includes a download button, next / prev buttons, and if we're not at the tippy toppy of hte page an "X" or close button.
we also respond to left, right, and esc for navigating.
next time we should get non-images working in here...!
Test Plan:
played with maniphest - looks good
made comments with images. looks good.
made sure multiple image comments worked.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T1896
Differential Revision: https://secure.phabricator.com/D3705
